Rangsh commented on PR #12173: URL: https://github.com/apache/seatunnel/pull/12173#issuecomment-6259510689
@SEZ9 pointers on `7948e2640` (no new push). Current PR file list is only: - `seatunnel-benchmarks/.../IMapJobStorageBenchmark.java` - `seatunnel-benchmarks/.../IMapJobGrowthBenchmarkWorkload.java` - `seatunnel-engine/.../JobHistoryService.java` - `seatunnel-engine/.../JobHistoryServiceFinishedMetricsTest.java` **F4 (docs)** — `docs/en|zh/engines/zeta/benchmark.md` are not in that file list. The growth-scenario notes added in `909f878` were removed in `9a6077c58`; this head does not touch either page. **F2 / F5 (`JobHistoryService`)** — `storeFinishedPipelineMetrics` (`JobHistoryService.java:267-289`): - Javadoc (`:267-277`): locks `jobId`, merges into the current IMap value, writes once; concurrent callers for the same `jobId` are serialized by the IMap key lock. - Body (`:280-288`): `finishedJobMetricsImap.lock(jobId)` / `unlock(jobId)` in `try/finally`. That is Hazelcast's cluster-wide pessimistic lock keyed by job ID, not a per-`JobMaster` / per-instance monitor. Inside the lock: `get` → merge (empty bag when absent) → one TTL `put`. **F1 / F3 (durability sample)** — the single-key reload is intentional. - A failed write-through append on a non-final iteration is not durability-sampled. Class Javadoc (`IMapJobGrowthBenchmarkWorkload.java:43-51`) and `verifyGrowthPhase` (`:160-166`) say it surfaces at trial tear-down, or earlier only if a resident-size check fails. - Sampling gate is `shouldSampleGrowthDurability()` (`:294-304`): mid-trial reload only when `growthIterationIndex == 0 && initialStoredJobCount == 0`. `initialStoredJobCount=1000` is trial tear-down only. A full WAL replay under that pressure OOMs the fork. - Tear-down does **not** cover the full last batch with one `loadAll`. `tearDown()` (`:192-196`) calls `verifyGrowthBatchDurability()` (`:341-356`), which reloads only the last key via `loadAll(Collections.singleton(key), true)` (`:358-396`). The other keys are resident checks only (`verify*BatchResident`). Full-batch widening is the separate follow-up, not this PR. **F6 (test)** — `JobHistoryServiceFinishedMetricsTest` asserts the stored value, not `never().computeIfAbsent` (that call does not appear in the test): - Observable merge: `storeFinishedPipelineMetricsMergesExistingFinishedMetrics` (`:112-131`) captures the single `put` and asserts both existing `SourceReceivedCount=1` and incoming `SinkWriteCount=2`. - Null path: `storeFinishedPipelineMetricsRejectsNullMetrics` (`:135-138`). - TTL: new-job (`:100-105`), merge (`:121-126`), and distinct-tag (`:155-160`) all verify `put(..., FINISHED_JOB_EXPIRE_MINUTES, MINUTES)` with `FINISHED_JOB_EXPIRE_MINUTES = 1440` (`:57`). New-job also verifies `lock(jobId)` / `unlock(jobId)` (`:107-108`). Happy for you to check these against `7948e2640`. I will not rebase or push unless a review asks for it. -- 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]
