Add YCSB benchmark module with YQL driver and two-pass runner#981
Open
Andrii Lomakin (andrii0lomakin) wants to merge 37 commits into
Open
Add YCSB benchmark module with YQL driver and two-pass runner#981Andrii Lomakin (andrii0lomakin) wants to merge 37 commits into
Andrii Lomakin (andrii0lomakin) wants to merge 37 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new YCSB benchmark module, including a forked core framework, a YQL-based driver, and automated runner scripts. It also modifies transaction handling to correctly propagate commit-time exceptions. The review identified critical bugs in the retry logic of the driver and workload where stateful iterators were reused, as well as a potential crash during field extraction from vertices.
Andrii Lomakin (andrii0lomakin)
force-pushed
the
ycsb
branch
from
b3506ab to
34de905
Compare
Set up the ycsb/ module for YCSB benchmarks: parent POM integration, dependencies (youtrackdb-core, HdrHistogram, Jackson 2.x, SLF4J/Log4j2, JUnit 4), shade plugin for uber-jar with Client main class, ErrorProne disabled, --add-opens JVM flags, coverage exclusion profile, and deploy skip.
Fork 11 foundation classes from YCSB core (site.ycsb) into com.jetbrains.youtrackdb.ycsb: Status, DBException, WorkloadException, ByteIterator family (5 classes), Utils, and generator base classes (Generator, NumberGenerator). Package rename only — no behavioral changes. Spotless-formatted to project conventions.
Fork 15 generator classes from YCSB core (site.ycsb.generator) into com.jetbrains.youtrackdb.ycsb.generator: Zipfian, ScrambledZipfian, Uniform, UniformLong, Sequential, Counter, AcknowledgedCounter, Discrete, SkewedLatest, Hotspot, Exponential, Histogram, Constant, File, and IncrementingPrintableString generators. Package rename and Utils import update only — no behavioral changes.
…rator Replace java.util.* wildcard import with explicit imports per project conventions (no wildcard imports, threshold 999).
Fork 11 measurement classes from YCSB core into com.jetbrains.youtrackdb.ycsb.measurements: Measurements, OneMeasurement (base + 4 implementations), TwoInOneMeasurement, and 4 exporter classes. Migrate Jackson 1.x (org.codehaus.jackson) to Jackson 2.x (com.fasterxml.jackson.core) in JSON exporters — same API surface, only package paths change.
…ateJsonGenerator Replace deprecated Jackson 1.x compatibility method createJsonGenerator() with the Jackson 2.x createGenerator() in both JSON measurement exporters.
Fork 9 YCSB framework classes + UnknownDBException: DB, DBWrapper, DBFactory, Workload, CoreWorkload, Client, ClientThread, StatusThread, TerminatorThread. Remove all HTrace distributed tracing from DBWrapper and Client (TraceScope, Tracer, HTraceConfiguration) while preserving measurement logic. Update all site.ycsb string literals to com.jetbrains.youtrackdb.ycsb. CoreWorkload placed in workloads subpackage.
Replace java.util.*, com.jetbrains.youtrackdb.ycsb.*, and com.jetbrains.youtrackdb.ycsb.generator.* wildcard imports with explicit imports per project conventions.
Fork 8 YCSB test classes (19 tests total) with package rename and TestNG→JUnit 4 conversion: TestByteIterator, TestStatus, TestUtils, AcknowledgedCounterGeneratorTest, TestIncrementingPrintableStringGenerator, TestZipfianGenerator, TestMeasurementsExporter, TestCoreWorkload. Convert TestNG annotations to JUnit 4 equivalents, swap assertEquals parameter order, convert expectedExceptions to expected. All 19 tests pass, uber-jar builds successfully with Client main class.
Fix three track-level code review findings: - CQ1: Fix stale @see Javadoc in OneMeasurementHistogram referencing wrong package path (missing .measurements subpackage) - BC1: Remove non-existent BasicDB default from Client — the class was not forked, so running without -db would NPE. Now prints a clear error message and exits. - TC1: Add test for JSONMeasurementsExporter verifying the Jackson 2.x createGenerator() migration produces valid JSON output with expected field structure.
Implement the YCSB DB binding for YouTrackDB using YQL. The driver manages a shared YouTrackDB instance and YTDBGraphTraversalSource across all client threads via a static ReentrantLock and AtomicInteger counter. init() creates the database (DISK or MEMORY), defines the usertable schema with 10 field properties and a unique index on ycsb_key, and opens a shared traversal source. cleanup() closes resources when the last client thread exits. insert() builds a dynamic CREATE VERTEX statement from the values map and executes it within executeInTx(). Remaining operations (read, scan, update, delete) return Status.NOT_IMPLEMENTED as placeholders.
Fix resource leak in init(): if openTraversal or createSchema throws, the partially-created YouTrackDB instance and traversal source are now closed via a try-finally guard (CQ1/BC1). Strengthen all insert tests to verify data persistence via direct YQL read-back queries instead of only checking Status.OK (TB1/TB2/TB3). Add package-private getTraversalSource() accessor for test use. Additional fixes: - Add scan() to unimplemented operations test (CQ3/TB4/TC5) - Add empty values map test for args array boundary (TC1) - Fix testCleanupAndReinit state leak by creating fresh client (TX1) - Add assertNull check for traversalSource after cleanup (TB5/TX3) - Add license header to test file (CQ4) - Add ytdb.dbtype to Javadoc properties list (CQ2) - Rename testInsertAndVerifyViaRead to testInsertAndVerifyDataPersisted
Implement read() using SELECT FROM with Vertex property extraction, supporting both fields=null (all properties) and specific field sets. Returns NOT_FOUND when no record matches the key. Implement update() with dynamic SET clause building, wrapped in executeWithRetry() — a bounded retry loop (3 attempts) that catches ConcurrentModificationException with random backoff for MVCC conflicts. Tests: insert-then-read round trips (all fields, specific fields), read non-existent key (NOT_FOUND), insert-then-update-then-read (verify updated + unchanged fields).
Performance: replace toList() with tryNext() in read() to avoid ArrayList allocation for single-result lookups. Use vertex.properties() iterator instead of keys()+value() double traversal in fields=null path. Bug fix: guard update() against empty values map which would produce malformed SQL. Empty update is now a no-op returning OK. Testing: make executeWithRetry() package-private and add unit tests for transient CME (succeeds after 2 failures) and persistent CME (exhausts all 3 retries). Add size assertion to testReadAllFields. Add testUpdateEmptyValuesMapReturnsOk. Documentation: add known limitation comment on executeWithRetry about finishTx() swallowing commit-time CME in executeInTx(). Fix misleading comments in read() and insert()/update() SQL patterns.
Implement delete() using DELETE VERTEX with executeWithRetry() for MVCC conflict handling, matching the same pattern as update(). Implement scan() using SELECT FROM with ORDER BY ycsb_key ASC and LIMIT clause. Supports both fields=null (all properties via vertex.properties() iterator) and specific field sets. Results are added to the Vector<HashMap> as required by the YCSB DB contract. Tests: delete-and-verify-not-found, scan-from-middle with ascending order verification, scan-with-specific-fields, scan-beyond-dataset (fewer records returned), scan-with-startkey-beyond-all-keys (empty result). Remove unimplemented operations test — all 5 ops are now implemented.
Extract duplicated vertex property extraction logic from read() and scan() into a shared extractFields() helper method. Remove unnecessary @SuppressWarnings("unchecked") from scan() that had no unchecked cast.
Concurrent init test: 4 threads call init() simultaneously via CountDownLatch, verifying exactly one database is created and all threads can insert records. After all 4 clients clean up, shared static state is null. Full workload round-trip test: CoreWorkload loads 100 records then runs 200 operations (50% read, 30% update, 10% scan, 10% insert) with uniform distribution. Exercises the driver through the real YCSB framework path including key generation, field selection, and operation distribution. Requires Measurements singleton initialization.
Fix double-cleanup vulnerability (TS1): move `client = null` immediately after early cleanup() calls in testCleanupAndReinit, testConcurrentInit, and testFullWorkloadRoundTrip to prevent tearDown from decrementing clientCount to negative values if an assertion fails between cleanup and null assignment. Fix false-confidence workload test (TB1): CoreWorkload.doTransaction() always returns true regardless of DB operation results. Replace the meaningless success-count assertion with post-workload data integrity verification (record count check + spot-check read-back). Add 3 new tests for untested paths: - testExecuteWithRetryRestoresInterruptOnInterruption (TC1): verify interrupt flag restoration during CME backoff - testInsertDuplicateKeySucceedsSilently (TC2): document that CREATE VERTEX with duplicate unique key does not throw - testExecuteWithRetryNoRetryOnNonCmeException (TC4): verify non-CME exceptions return ERROR immediately without retry Strengthen assertion depth in existing tests (TB2-TB5): - testDeleteAndVerifyNotFound: verify field content before delete - testScanFromMiddle: verify field values, not just key order - testScanBeyondDataset: verify key and field content - testConcurrentInit: verify all 4 clients can insert Clean up fully qualified type names (CQ1): add proper imports for AtomicInteger, CountDownLatch, TimeUnit, ConcurrentModificationException, RecordId, Measurements, CoreWorkload.
Create workloads/ directory with workload-common.properties (shared defaults: CoreWorkload FQCN, YQL driver, 3.5M records, 10x100B fields, HdrHistogram) and five per-workload files (B, C, E, W, I) that override only operation proportions and distribution settings. Add src/main/resources/project.properties with Maven-filtered version to suppress "Unable to retrieve client version" stderr noise. Configure resource filtering in pom.xml for project.properties only. Add WorkloadPropertyFilesTest (8 tests) verifying: common properties contain all required fields, each workload produces the correct operation mix via DiscreteGenerator sampling, all workloads use zipfian distribution, and per-workload overrides take effect.
…erty Move requestdistribution=zipfian from all 5 per-workload files into workload-common.properties to eliminate duplication. Tighten statistical tolerance bands from +/-10% to +/-3% of expected proportion (6-sigma safe at 10K samples). Add assertRatio helper. Add UPDATE ratio assertions for workloads B and W. Load common properties directly (not merged) in commonPropertiesHaveRequiredFields to catch accidental shadowing. Add exact-value verification for all workload proportions. Add proportions-sum-to-1.0 guard. Add project.properties Maven filtering test. Use Path.of over Paths.get.
Create ycsb/run-ycsb.sh with: argument parsing for all options (workloads, record-count, operation-count, threads, throughput-ratio, db-path, results-dir, skip-build, skip-load, help), CPU-count detection with Linux/macOS fallback, JVM --add-opens flags array matching pom.xml argLine, uber-jar resolution from Maven target, run_ycsb() helper function for invoking the uber-jar with proper arguments and exit code checking, SIGINT/SIGTERM trap with recovery instructions, and Maven build step (skippable). This creates the script framework; load, snapshot, and workload execution logic will be added in subsequent steps.
Fix critical bug: translate run_ycsb() mode from "run" to "-t" (YCSB CLI uses -t for transaction mode, not -run). Add mode validation. Add -Xms/-Xmx heap flags (default 4g) to match pom.xml test config and prevent OOM on default 3.5M record dataset. Add --heap CLI option. Add require_arg() guard for all value-bearing options to prevent cryptic "unbound variable" errors. Add require_positive_int() for numeric options. Add workload name validation against available property files. Add non-empty check for --results-dir. Fix PIPESTATUS reliability: restructure tee pipeline to capture exit code outside of if-negation. Fix signal handler to expand DB_PATH instead of printing literal placeholder. Use portable head -n 1 instead of GNU-only find -quit.
Add load phase to run-ycsb.sh: invokes YCSB in -load mode with ytdb.newdb=true to create a fresh database, records wall-clock time, then creates a directory snapshot via cp -a for reproducible workload runs. The snapshot is taken after the YCSB JVM exits, which implicitly flushes WAL via database close. The --skip-load flag skips both loading and snapshot creation, but validates that a snapshot exists. Existing database and snapshot are cleaned up before a fresh load.
Fix blocker: change ytdb.path to ytdb.url to match the actual YouTrackDBYqlClient URL_PROPERTY constant. The wrong property name caused the database to be created at the default path instead of the user-specified --db-path. Add post-load existence check for DB_PATH before attempting snapshot, with a diagnostic message pointing to ytdb.url if the directory is missing. Use atomic copy-then-rename for snapshots: cp -a to a temporary directory, then mv to the final path. This prevents partial snapshots from surviving interrupted runs and being mistakenly used with --skip-load. Strip trailing slash from DB_PATH to prevent snapshot path corruption (e.g., /data/ytdb/ would produce /data/ytdb/-snapshot as a child directory). Add non-empty validation for DB_PATH. Log snapshot path in the configuration summary.
Add workload execution loop to run-ycsb.sh: for each selected workload,
restore database from snapshot, run pass 1 (max throughput with
-target 0), parse throughput from YCSB output, compute fixed target
(throughput * ratio), restore again, run pass 2 (fixed throughput with
measurement.interval=intended for coordinated-omission-corrected
latency).
Throughput parsing uses awk on the [OVERALL], Throughput(ops/sec) line.
If throughput is not a positive number, pass 2 is skipped with a
warning. Target computation uses integer truncation via awk printf %d.
Each pass output is saved to $RESULTS_DIR/workload-X-pass{1,2}.txt.
Both passes use ytdb.newdb=false to preserve the snapshot-restored data.
… workload failure Use awk -v flag to pass throughput and ratio as awk variables instead of interpolating shell variables into awk program strings, preventing potential code injection from malformed YCSB output. Add --throughput-ratio validation: must be a positive number in (0, 1.0]. Previously no validation existed (other numeric args were validated). Add empty-TARGET guard for robustness. Wrap run_ycsb calls in if-conditionals so that YCSB process failures skip to the next workload instead of aborting the entire script. With set -e, bare run_ycsb calls would terminate the script on failure, losing results from remaining workloads. Remove duplicate mkdir -p for results directory. Add inline comment explaining measurement.interval=intended for coordinated-omission correction.
Add post-workload summary section to run-ycsb.sh: parses throughput from pass 1 and latency percentiles (P50, P95, P99) from pass 2 for each workload's primary operation (READ for B/C/W, SCAN for E, INSERT for I). Prints a formatted summary table to stdout and writes a structured summary.json to the results directory. Latency parsing uses awk with -v variable binding for safe field extraction from YCSB's [METRIC], MeasurementName, value output format. Missing results (failed workloads) display as "-" in the table and -1 in JSON.
Change sentinel for missing metric values from "-" (non-empty string
that defeats ${VAR:--1} substitution) to empty string. This ensures
JSON output uses -1 for missing values instead of bare "-" which is
invalid JSON.
Update parse_latency to return empty string instead of "-" when the
metric is not found, aligning with parse_throughput's convention.
Display table uses ${VAR:--} for human-readable dash. Update
primary_operation comment to document W→READ rationale.
…in sum test The allWorkloadProportionValuesAreExact test now checks all five proportions per workload (including zeros) to guard against accidental removal of zero-proportion lines that suppress CoreWorkload defaults. The allWorkloadProportionsSumToOne test now uses the same defaults as CoreWorkload.createOperationGenerator() so a missing proportion line produces a sum mismatch instead of silently falling back to "0".
Documents the new ycsb benchmark module: purpose (storage-level benchmarking via YQL, distinct from jmh-ldbc's graph focus), the five named workloads (B/C/E/W/I) with proportions and distribution, the two-pass execution model with coordinated-omission correction, three execution paths (run-ycsb.sh, full build, manual uber-jar), all six driver properties including the ytdb.newdb=true trap, and the summary.json schema with -1 sentinel.
Address step-level review findings: - BC1/BC2: manual uber-jar example now instructs to run from repo root, warns about the jar glob, and shows recordcount/operationcount overrides so users don't accidentally load 3.5M records. - BC3: --skip-load note clarifies the snapshot is the source of truth and $DB_PATH is overwritten at the start of every pass. - BC4: concurrent-run warning now covers the default $RESULTS_DIR/ytdb-data location and explains when distinct results-dir values are sufficient. - CQ1: note that p99.9 is present in the raw pass-2 text output even though summary.json only rolls up p50/p95/p99. - CQ2/CQ3: drop brittle line-number references and the hard-coded count of --add-opens flags; reference the canonical arrays by name instead. - CQ5: mark the "~3.5 GB" figure as approximate. - CQ6: correct prerequisite from "POSIX shell (bash)" to "Bash 4+".
Post-implementation artifacts for the YCSB benchmark module: - design-final.md: actual design reflecting implemented code (class diagrams, workflow sequences, thread safety, snapshot/ restore, two-pass execution — cross-referenced with real file and line numbers). - adr.md: architecture decision record with actual outcomes. Retains original D1–D5 and adds D6–D9 for decisions that emerged during execution (binding sub-package, atomic snapshotting, graceful per-workload failure propagation, configurable database type). Aggregates key discoveries from both step and track episodes for future work.
The YAML frontmatter was only meaningful during the in-flight workflow; now that the module ships, keep the human-readable content and link the README to the final design document.
The final design doc froze a snapshot of the code at ship time; keeping it alongside an evolving implementation would only create drift. Drop it and the now-dangling README reference.
The benchmark runner packages an uber-jar to execute the YCSB suite; it is not a code-style gate. Spotless' Equo P2 offline cache is also prone to transient NoSuchFileException on bundle-pool jars, which aborted the build before any benchmark could run. Skip the check so benchmark runs are decoupled from formatter cache state.
finishTx used to catch every exception from tx.commit() and only log it, so commit-time failures — notably ConcurrentModificationException raised by MVCC inside AbstractStorage.doUpdateRecord — were silently swallowed. Drivers built on executeInTx / computeInTx (e.g. the YCSB YQL client, whose executeWithRetry wraps update and delete) could not see the conflict and therefore dropped the update instead of retrying. Rethrow commit exceptions from finishTx so callers can react. Keep rollback failures logged but not rethrown: the body exception is the primary cause and must not be masked by a secondary cleanup error. Add YTDBTransactionFinishTxTest covering happy path, body-exception rollback, and commit-time CME propagation for both executeInTx and computeInTx. The CME tests bump the record version from a background thread via a direct DatabaseSessionEmbedded (a second YTDBGraph would close the shared cached SessionPool on its close() and invalidate the main-thread session). Also drop the now-outdated "known limitation" notes in the YCSB driver Javadoc and the YCSB ADR.
ByteIterator.toString() consumes the underlying stream on the first call (e.g. RandomByteIterator), so any retry that re-reads the source map writes empty strings to the database. * YouTrackDBYqlClient.update() now snapshots values into a Map<String, String> before calling executeWithRetry, so CME retries see stable data. * CoreWorkload.doInsert() snapshots values up-front, so the insertion retry loop does not feed exhausted iterators to the driver. * YouTrackDBYqlClient.extractFields() guards each property lookup with VertexProperty.isPresent() to avoid NoSuchElementException when a requested field is unset on the vertex. Added deterministic regression tests for all three issues: a RetrySimulatingClient that double-invokes the retryable operation to observe toString() call counts, a partial-vertex read that would previously throw, and a stub DB that captures per-attempt values to prove retry snapshots match the first attempt.
Andrii Lomakin (andrii0lomakin)
force-pushed
the
ycsb
branch
from
34de905 to
e96ee45
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
YouTrackDB has JMH benchmarks for graph traversal (
jmh-ldbc) but no coverage of the storage-level I/O paths — read, update, insert, scan, delete — that sit underneath the query engine. Without this, regressions in the page cache, WAL, and index paths are invisible until they surface in production or in traversal benchmarks where they are entangled with MATCH-engine work.This PR adds a dedicated
ycsb/Maven module that forks the YCSB core framework, ships a YQL-based driver, and provides a one-command runner so engineers can quantify storage throughput and latency in isolation.Summary
YouTrackDBYqlClientdriver — exercises storage paths directly via simple parameterized YQL (SELECT/UPDATE/INSERT/DELETE), not the Gremlin/MATCH layer. Runs YouTrackDB embedded — no server needed.B(95/5 read/update),C(read-only),E(scan-heavy),W(50/50 read/update),I(insert-burst, 20/80 read/insert). Workloads A/D/F from upstream YCSB are intentionally omitted.run-ycsb.sh) — build → load → snapshot → pass 1 (-target 0, max throughput) → restore → pass 2 (fixed throughput at80%of pass 1 withmeasurement.interval=intendedfor coordinated-omission correction). Emits a summary table andsummary.json.ycsb/README.md(~240 lines) covers prerequisites, quick start, three execution paths, workload parameters, and the JSON output format.gremlin/finishTxnow propagates commit exceptions instead of swallowing them (YTDBTransactionFinishTxTest).Scope notes
YouTrackDBYqlClientare excluded from JaCoCo via thecoverageprofile inycsb/pom.xml. Rationale: the forked framework is upstream code, and the driver is verified by integration tests rather than line coverage.docs/adr/ycsb/adr.mdfor the full architecture decision record.Test plan
./mvnw -pl ycsb clean test— unit + integration tests pass (ported YCSB tests + driver tests)./ycsb/run-ycsb.sh --record-count 1000 --operation-count 500 --workloads B,C— smoke run completes end-to-end and emitssummary.json./mvnw clean package -DskipTests— full build succeeds with the new module in the reactordevelop