JingsongLi commented on PR #10133:
URL: https://github.com/apache/paimon/pull/10133#issuecomment-5832291615

   Reviewed latest head `da05812` for production. The linked DV lag issue gives 
this feature clear end-to-end value; the latest commit addresses the previously 
reported per-bucket timer retention and adds worker-churn/shared-pool tests. 
Local JDK 8 verification passed: four focused classes (11 tests), existing 
compaction/write classes (26 tests), and normal `paimon-api,paimon-core` 
compile with Spotless/Checkstyle.
   
   Two merge blockers remain:
   
   1. **P1 — generated configuration docs are missing.** Both Core CI jobs fail 
in `ConfigOptionsDocsCompletenessITCase`: `Option compaction.task-threads in 
class org.apache.paimon.CoreOptions is not documented.` The prose docs were 
updated, but `docs/generated/core_configuration.html` was not regenerated. 
Please regenerate it per `paimon-docs/README.md` and rerun CI. `git diff 
--check` also reports trailing whitespace in 
`docs/docs/primary-key-table/table-mode.md:109`.
   
   2. **P1 — timer retirement can race with the next reporter on a shared 
worker.** In `ReporterImpl.getCompactTimer()`, 
`compactTimers.computeIfAbsent(threadId, ...)` happens before 
`acquireCompactTimer(threadId)`, while `unregister()` can remove the timer in 
between. A concrete interleaving: bucket B obtains bucket A's worker timer, 
bucket A unregisters and removes the last reference, then B acquires the 
reference and calls `start()` on the now-detached timer. 
`CompactTask.stopTimer()` calls `getCompactTimer()` again, which creates a 
fresh timer and calls `finish()` without a matching `start()`; 
`CompactTimer.finish()` throws `IllegalArgumentException`. This is reachable 
with the fixed pool/default shared worker when an idle bucket writer is retired 
while another bucket starts compaction. Make timer lookup/acquisition/removal 
atomic for a thread ID, and add a concurrent retirement/start regression test.
   
   The option is worth keeping open, but these issues need fixing before 
production merge.
   


-- 
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