HDDS-15680. Cache per-block EC checksum pipeline within a file#10709
Open
yandrey321 wants to merge 7 commits into
Open
HDDS-15680. Cache per-block EC checksum pipeline within a file#10709yandrey321 wants to merge 7 commits into
yandrey321 wants to merge 7 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-file XceiverClient reuse buildChecksumPipeline now derives the STANDALONE pipeline ID from the sorted UUIDs of the selected nodes via UUID.nameUUIDFromBytes. Files that land on the same EC placement group produce an identical pipeline ID, so XceiverClientManager can reuse the same gRPC connection across files (cross-file reuse, Fix 2). Pipeline.newBuilder() is used instead of ecPipeline.toBuilder() because toBuilder copies the EC nodeStatus map; the resulting size mismatch caused Pipeline's internal consistency check to fall back to a random ID, which defeated the determinism. Benchmark (5 threads, RS-3-2, 1-block files): Fix 3 only (random ID per file): ~800 files/s at 5ms, ~412 files/s at 10ms Fix 2+3 (deterministic ID): ~94K files/s at 5ms, ~95K files/s at 10ms Speedup: 117x at 5ms DN latency, 231x at 10ms DN latency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-3-only vs Fix-2+3 scenarios, fix Mockito contention - Consolidate duplicate constants: WARMUP_SECS, MEASURE_SECS, LATENCIES_MS, MAX_POOL_SIZE (one constant replaces each pair) - Restore runBlockCacheBenchmark scenarios to the meaningful comparison: Scenario A = Fix 3 only (per-file unique pipeline, intra-file reuse, NewConn/file = 1.00); Scenario B = Fix 2+3 (cross-file + intra-file reuse, NewConn/file = 0.00). This shows the latency-driven speedup clearly. - Apply withSettings().stubOnly() to all hot-path Mockito mocks. Without stubOnly(), Mockito records every invocation via a global synchronized SequenceNumber and captures the full call stack via StackWalker on each call. At 0ms DN latency with 5 threads sharing a pool-hit XceiverClient mock, this caused severe serialization and f.get() blocking for minutes. stubOnly() skips invocation recording, eliminating both bottlenecks. - Add buildRandomKeyInfoMultiBlock for Scenario A (N-block files with a fresh unique EC pipeline per file). Refactor buildRandomKeyInfo to delegate. - Add -Dsurefire.fork.timeout=3600 to run instructions (default 1200s is less than the benchmark's ~25 min runtime). Benchmark results (5 threads, 10s warmup + 20s measurement): RS-3-2 5ms: A=804 files/s (NewConn/file=1.00) vs B=200K files/s (0.00) -> 249x RS-3-2 10ms: A=413 files/s (NewConn/file=1.00) vs B=194K files/s (0.00) -> 470x RS-6-3 5ms: A=698 files/s (NewConn/file=1.00) vs B=178K files/s (0.00) -> 256x RS-6-3 10ms: A=361 files/s (NewConn/file=1.00) vs B=179K files/s (0.00) -> 496x CacheHit% = 0/50/66/80% for 1/2/3/5 blocks (exactly (N-1)/N, Fix 3 verified). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
What changes were proposed in this pull request?
When computing a file checksum for an erasure-coded key, ECFileChecksumHelper builds a temporary STANDALONE pipeline (replica index 1 + parity nodes) for every block. All blocks of a file share the same EC placement group, so that pipeline is identical for every block — rebuilding it per block (node filtering, deterministic-ID hashing, allocation) is wasted work that grows with the number of blocks.
This PR caches the standalone pipeline per placement group inside the helper, so it is built once per file (on the first block) and reused for the rest. It builds on HDDS-15643 (#10594) — already on master — which removed the redundant OM lookupKey RPC and made the pipeline ID deterministic for cross-file connection reuse. This adds the intra-file reuse on top. It is purely client-side CPU savings: no wire, OM/SCM, or checksum-result changes.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15680
How was this patch tested?
New unit tests in TestFileChecksumHelper cover the cache hit/miss paths, the node filtering, and the acquire-failure path.
Benchmarked with FileChecksumBenchmark#runBlockCacheBenchmark (5 threads, each checksumming random-placement EC files), comparing the cache off vs on while holding everything else equal.
Results — throughput gain (cache on ÷ off), by file size in blocks
Single-block files show no change (nothing to cache — the 1-block row is a control and sits within ~2-3% noise). The gain grows with file size as the cache eliminates the redundant per-block pipeline rebuilds — up to ~6–8% for large multi-block EC files, slightly higher for wider EC schemes (RS-6-3). The improvement is client-side CPU only and does not change the checksum result.
As a summary the fix would have slight performance penalty for small files, and single digit improvement for files spans to multiple blocks.