Rangsh commented on PR #12173:
URL: https://github.com/apache/seatunnel/pull/12173#issuecomment-6306068566
Thanks @nzw921rx, and thanks @DanielLeens for reading `b137173f7` and
spelling out the split. Agreed. I will not treat the current table as proof.
Two points from that read that I am taking as given:
1. `runningJobGrowth / initialStoredJobCount=1000` cannot be moved by the
`JobHistoryService` change. `appendRunningJobBatch()` only writes the three
running-job maps. `storeFinishedPipelineMetrics` and `storeFinishedJobState`
are both called only from `appendCompletedJobLifecycleBatch()`, so the
production change can only matter for `completedJobHistoryGrowth`. The +510.63%
CV on the running-job row is a fixture/measurement question, not a
lock-and-merge question.
2. "Timed method body is byte-identical" was too narrow.
`prepareGrowthBatch()` now uses `GROWTH_KEY_BASE + growthBatchSequence++ * 100
+ index` instead of re-putting the same 100 keys after delete. That setup is
off the clock, but it changes what the timed `put`s hit. The fixture candidates
are both the per-iteration `loadAll` removal and the unique-key change.
I will follow @DanielLeens's order, not push a diagnostic head first:
- Sync this branch with current `dev` first (merge or rebase — say which you
prefer).
- Because `benchmarks.yml` compares `dev` with one `refs/pull/<n>/head` per
dispatch, this is two dispatches, not one three-way run:
- **B:** `dev` + only `IMapJobStorageBenchmark` and
`IMapJobGrowthBenchmarkWorkload`
- **C:** B + `JobHistoryService` and
`JobHistoryServiceFinishedMetricsTest` (the full PR)
- **A** is the `dev` baseline of each dispatch.
If B already shows the CV jump on `runningJobGrowth / 1000`, the production
change is out of that row. If B is stable, C answers
`completedJobHistoryGrowth`. I will not add the B1/B2 split (fixed keys vs
unique keys) unless B shows the jump and you want that next. Please compare the
raw per-run numbers, not only the medians. `-prof gc` only if B still jumps
after the sync.
@nzw921rx — I will not move `b137173f7` until you confirm this is the run
you want, since only a maintainer can dispatch Benchmarks on
`apache/seatunnel`. Once you confirm, I will sync and post the B and C SHAs
here.
--
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]
