Rangsh commented on issue #12058: URL: https://github.com/apache/seatunnel/issues/12058#issuecomment-5643024886
@SEZ9 @DanielLeens posting the first single-variable candidate before any A/B, per https://github.com/apache/seatunnel/issues/12058#issuecomment-5642813148 and https://github.com/apache/seatunnel/issues/12058#issuecomment-5635786970. ### Clarification on #12081 vs this issue **#12081 does not solve / close #12058.** It remains **Related only**. - #12081 lands multiple independent correctness changes (`RequestFuture`, `WALWorkHandler` fail-close / survivability, `FileMapStore` durability visibility, `deleteAll` double-publish cleanup) **plus** the `HdfsWriter.flush()` sync-collapse. - That is **more than one variable**, so a before/after of the full PR cannot be treated as evidence that the accepted root cause (write-through wall wait) is fixed. - The PR body already states merging it must **not** close this issue; sync-collapse stays a **working hypothesis** until isolated on this same host with flame evidence. I will keep #12081 on the correctness track and run the #12058 A/B as a **separate, single-variable experiment** on top of unchanged `origin/dev` (or an equivalent one-hunk branch), not as “merge #12081 ⇒ CV fixed.” ### 1. Exact candidate (one variable only) + expected wall-frame shrink **Candidate:** change **only** `HdfsWriter.flush()` so each branch performs **exactly one** `hsync`-family durable sync and does not fall through into a trailing `hflush()` / second sync. - File / method: `imap-storage-file` → `HdfsWriter.flush()` - Not included in this A/B: `RequestFuture`, `WALWorkHandler`, fail-close / `FileMapStore` surfacing, batch-deadline, `deleteAll` publish cleanup, or any MapStore config change **Wall stack expected to shrink (if the hypothesis holds):** - Primary (benchmark / timed path): `updateOverview` → `MapProxyImpl.compute` → `InvocationFuture.get` → `LockSupport.park` / `Unsafe.park` (the ~60% wait hotspot from the baseline flames) - Secondary (WAL worker, off the JMH thread): `HdfsWriter.flush` / `FSDataOutputStream.hsync` Honest expectation for **this** harness (`fs.defaultFS: file:///`, LocalFileSystem): the local path is `hsync` + `hflush` → both effectively buffer `flush()` into the OS page cache (not multi-device fsync). So park / CV movement may be **small or null** here; the larger multi-sync waste is on a real HDFS cluster. A null or weak result on `file:///` is still useful: it would falsify “redundant sync count is what drives this benchmark’s park variance” without claiming a CV win. ### 2. Semantic invariants (unchanged) - Hazelcast MapStore stays **write-through** (`write-delay-seconds: 0` / shipped default) — **no** MapStore mode change, **no** write-behind. - Persistence stays enabled — **no** disabling MapStore / swapping in a no-op store for overview maps. - Every successful WAL APPEND still completes only after **exactly one** `hsync`-family call on the chosen branch (durability intent preserved; not fire-and-forget). - The benchmark thread still **waits** for write-through completion (`InvocationFuture` / `RequestFuture`) — nothing that only hides the wait (async MapStore, dropping the wait, measuring a different path). ### 3. Profiling confirmation I will re-run on the **same** Corretto **11.0.26** / Apple M1 setup, same initial data and workload as the baseline flames already attached on this issue: ```bash bash tools/benchmarks/profile_benchmarks.sh profile wall \ --repository . \ --benchmark 'CheckpointStorageBenchmark.checkpointOverviewIncrementalUpdate$' bash tools/benchmarks/profile_benchmarks.sh profile cpu \ --repository . \ --benchmark 'CheckpointStorageBenchmark.checkpointOverviewIncrementalUpdate$' bash tools/benchmarks/profile_benchmarks.sh profile gc \ --repository . \ --benchmark 'CheckpointStorageBenchmark.checkpointOverviewIncrementalUpdate$' ``` I will attach the new `flame-wall-*.html` / `flame-cpu-*.html` (and GC summary) **alongside** the baseline ones so the hotspot delta is visible. **Score / Error / CV alone will not be treated as evidence.** Holding the A/B until this candidate + invariants look right to you. -- 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]
