Rangsh commented on issue #12058:
URL: https://github.com/apache/seatunnel/issues/12058#issuecomment-5650911350

   @DanielLeens @SEZ9 reporting the isolated single-variable A/B you authorized 
— **either outcome**, before any production CV conclusion.
   
   ### Contract followed
   
   - Exact current-dev baseline: `origin/dev` @ `fbc0524d4`
   - **Only** `HdfsWriter.flush()` control-flow change (one hsync-family call / 
branch, no trailing `hflush`)
   - Excluded: RequestFuture, WAL failure handling / fail-close, 
batch-deadline, MapStore, CheckpointCoordinator
   - Write-through + caller completion wait preserved
   - `file:///` result labelled **filesystem-specific**
   - Branch call-path coverage: `HdfsWriterFlushCallCountTest` green 
(method-call-count only; **not** disk-sync / crash-durability proof)
   - Same-environment wall / CPU / GC + raw per-fork + Score/Error/CV
   - Conclusion compares original `InvocationFuture.get`/park hotspot vs 
storage sync path — not aggregate scores alone
   - **#12081 stays Related only**; this is not evidence that the full PR 
resolves #12058
   
   Artifacts: https://gist.github.com/Rangsh/66008c5bad44fb05825db40198349175 
(`AB-REPORT.md` + before/after JMH JSON, profiles, flames)
   
   ### Environment
   
   | Item | Value |
   | --- | --- |
   | Machine | Apple M1 / macOS 14.7.x |
   | JDK | Corretto **11.0.26** |
   | Method | `CheckpointStorageBenchmark.checkpointOverviewIncrementalUpdate$` 
|
   | JMH | existing defaults (`SingleShotTime`, 3 forks, 3/5 
warmup/measurement) |
   | Store | write-through MapStore, `fs.defaultFS: file:///` |
   
   ### Score / Error / CV
   
   | Side | Score ± Error (us/op) | CV (n=15) | min–max |
   | --- | ---: | ---: | --- |
   | BEFORE (multi-sync + trailing hflush) | 173.607 ± 61.793 | **32.2%** | 
129.3–376.0 |
   | AFTER (one hsync-family / branch) | 180.675 ± 69.126 | **34.6%** | 
116.4–350.0 |
   
   **BEFORE raw:** fork0 `[171.0, 129.3, 177.0, 144.2, 178.7]` · fork1 `[151.5, 
154.8, 376.0, 138.4, 165.8]` · fork2 `[178.4, 163.1, 158.3, 160.4, 157.3]`
   
   **AFTER raw:** fork0 `[163.2, 154.6, 158.6, 291.2, 116.4]` · fork1 `[350.0, 
250.3, 157.6, 161.6, 137.7]` · fork2 `[130.3, 156.3, 158.8, 154.5, 168.8]`
   
   ### Hotspot comparison (wall / CPU / GC)
   
   - **Wall:** both sides still show the timed path as wait-dominated: 
`updateOverview` → `MapProxyImpl.compute` → `AbstractInvocationFuture.get` / 
`LockSupport.park`, with WAL completion via `RequestFuture` / 
`HdfsWriter.flush` → `hsync` off the JMH thread. The original 
park/`InvocationFuture.get` shape **does not disappear** after the flush 
collapse on `file:///`.
   - **CPU:** `hsync`/`HdfsWriter.flush` remains ~**0.5%** of CPU samples 
(BEFORE ~0.48%, AFTER ~0.53%) — still not a CPU-hot sync.
   - **GC:** Alloc/op ~286.6–286.7 KB/op; GC count 1 both sides (5 ms → 4 ms) — 
no meaningful delta.
   
   ### Outcome (no production conclusion)
   
   On this **file:///** LocalFileSystem harness, the flush-only change produced 
a **null / weak (slightly worse) Score/CV movement** while the wait hotspot 
remained. Per your contract that is **useful evidence**: it does **not** 
support “redundant sync count is what drives this benchmark’s park variance on 
LocalFileSystem.”
   
   Same-process visibility / mocked call counts are still not crash-durability 
or device-sync-count proof. Real-HDFS multi-sync waste may still exist and is 
out of scope for claiming a CV win from this local run.
   
   **Not proposing a production CV fix from this A/B.** Happy to discuss the 
next single-variable mechanism step on unchanged-dev evidence (wait-path / 
harness / real-HDFS isolation) separately from #12081 correctness.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to