FrankChen021 commented on code in PR #20073:
URL: https://github.com/apache/druid/pull/20073#discussion_r3813153133
##########
server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java:
##########
@@ -604,11 +617,14 @@ private Runnable createWeakEntryReleaseRunnable(
weakCacheEntries.computeIfPresent(
weakEntry.cacheEntry.getId(),
(cacheEntryIdentifier, weakCacheEntry) -> {
+ if (weakCacheEntry != weakEntry || weakCacheEntry.isHeld()) {
+ // Someone else's entry, or someone else is still using ours;
either way, theirs to clean up.
+ return weakCacheEntry;
+ }
// If we never successfully mounted, go ahead and remove so we
don't have a dead entry.
// Furthermore, if evictImmediatelyOnHoldRelease is set, evict
on release if all holds are gone.
final boolean isMounted = weakCacheEntry.cacheEntry.isMounted();
- if ((isNewEntry && !isMounted)
- || (areWeakEntriesEphemeral && !weakCacheEntry.isHeld())) {
+ if ((isNewEntry && !isMounted) || areWeakEntriesEphemeral) {
Review Comment:
[P2] Failed partial entries can lose restart metadata
A failed mount consumes the onUnmount hook and deletes the info file. If
another hold remains, this guard retains the unmounted entry; the final
non-creator hold does not remove it. A later acquire reuses it without
rewriting the info file or restoring the hook, so bootstrap can lose the
segment after restart.
##########
server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java:
##########
@@ -604,11 +617,14 @@ private Runnable createWeakEntryReleaseRunnable(
weakCacheEntries.computeIfPresent(
weakEntry.cacheEntry.getId(),
(cacheEntryIdentifier, weakCacheEntry) -> {
+ if (weakCacheEntry != weakEntry || weakCacheEntry.isHeld()) {
Review Comment:
[P1] Replacement entry can be mistaken for the in-flight entry
If an abandoned mount entry is reclaimed and a replacement with the same ID
is registered, the mount verification and reservation adjustment check only the
ID. The old mount can therefore commit against the replacement's reservation
and leave its mapper and files live. Verify object identity before committing
or adjusting.
--
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]
