bitflicker64 commented on code in PR #3164:
URL: https://github.com/apache/hugegraph/pull/3164#discussion_r3990841084
##########
hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/business/BusinessHandlerImpl.java:
##########
@@ -1415,10 +1422,27 @@ public boolean dbCompaction(String graphName, int id,
String tableName) {
log.info("Partition {} dbCompaction started", id);
if (tableName.isEmpty()) {
lock(path);
- setState(id, doing);
- log.info("Partition {}-{} got lock, dbCompaction
start", id, path);
- op.compactRange();
- setState(id, compactionDone);
+ ReentrantLock rangeLock =
+ compactionRangeLock.computeIfAbsent(id,
+ k ->
new ReentrantLock());
+ if (!rangeLock.tryLock()) {
Review Comment:
⚠️ A full compaction that starts while a snapshot save holds this lock is
dropped, and the event-driven callers never retry it.
The comment says the next scheduled or triggered compaction will retry, but
these requests are one-shot: `handleCleanOp` after a data clean
(`PartitionEngine.java:1017`), `destroyPartition` when graphs remain
(`HgStoreEngine.java:505`), the PD `DB_COMPACTION` instruction
(`PartitionEngine.java:1263`), and the `/compat` REST call
(`PartitionAPI.java:207`), which has already told the operator the compaction
was submitted. The only periodic caller is the daily TTL cleaner, and it
compacts only after it cleaned something. If a periodic snapshot on the same
partition is between `tryLockCompactionRange` and `unlockCompactionRange`
(`SnapshotHandler.java:96-129`: checkpoint plus the checksum pass over every
file), the request leaves one INFO line. The cleaned range stays uncompacted
and the post-compaction `SYNC_BLANK_TASK` snapshot never runs. Before this
change a snapshot never cancelled a compaction.
Requested change: the snapshot side already fails fast, so this side can
wait. Use `rangeLock.tryLock(timeout, unit)` with a bound (this thread already
waits up to `timeoutMillis` in `lock(path)` above), and skip with a WARN only
if the save is still running when it expires. That cannot deadlock:
`onSnapshotSave` only calls non-blocking `tryLock()` and never takes
`pathLock`. Please also fix the comment.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
