SEZ9 commented on PR #12173: URL: https://github.com/apache/seatunnel/pull/12173#issuecomment-5642749281
Thanks @Rangsh for the detailed walkthrough against `c553a8c04`. 1. **F1 / F3 (durability sample)** — The Javadoc note on the resident-only window and the follow-up issue you linked sound reasonable. I'll re-check the `IMapJobGrowthBenchmarkWorkload` / `IMapJobStorageBenchmark` changes in `c553a8c04` before closing these, since the core concern in F1 is that a failed write-through append still doesn't fail the measured put for non-final iterations. 2. **F2 (`storeFinishedPipelineMetrics` atomicity)** — `lock(jobId)` / `unlock(jobId)` in `try`/`finally` around `get → merge → put` is the shape I was after. I'll verify it in `ca82f95`. 3. **F4 (docs)** — Understood that the growth methodology notes were removed from `docs/en|zh/engines/zeta/benchmark.md` in `9a6077c58` and that per-scenario detail stays in Javadoc. Fine with that once I've confirmed the diff. 4. **F5 (Javadoc)** — A contract-only description (lock + merge + single TTL `put`) is what I wanted; will confirm in `ca82f95`. 5. **F6 (tests)** — Your message appears to have been cut off after "asserts observable IMap `put` / merge / nul". Could you finish that point and confirm whether `JobHistoryServiceFinishedMetricsTest` in `ca82f95` covers the null-metrics path and the TTL-preserving `put` path? Once I've gone through the commits above and F6 is clarified, I don't expect anything else to remain open from the previous review. <!-- streview-comment:973 --> -- 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]
