Skip to content

Commit b1198bc

Browse files
committed
Prevent parallel file lock clear
FDBDirectory: When file lock clear is called in parallel by multiple threads, synchronically waiting for the first thread's clear to complete seems to be a better strategy. Resolves #3713
1 parent bb93d20 commit b1198bc

File tree

1 file changed

+29
-25
lines changed

1 file changed

+29
-25
lines changed

fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryLockFactory.java

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -210,40 +210,44 @@ private void fileLockCheckNewLock(byte[] val, long nowMillis) {
210210
}
211211

212212
private void fileLockClearFlushAndClose(boolean isRecovery) {
213-
Function<FDBRecordContext, CompletableFuture<Void>> fileLockFunc = aContext ->
214-
aContext.ensureActive().get(fileLockKey)
215-
.thenAccept(val -> {
216-
synchronized (fileLockSetLock) {
213+
synchronized (fileLockSetLock) {
214+
// If called by multiple threads, it seems to be a good idea to block all and allow one of them to do the actual closing.
215+
if (closed) {
216+
return;
217+
}
218+
Function<FDBRecordContext, CompletableFuture<Void>> fileLockFunc = aContext ->
219+
aContext.ensureActive().get(fileLockKey)
220+
.thenAccept(val -> {
217221
UUID existingUuid = fileLockValueToUuid(val);
218222
if (existingUuid != null && existingUuid.compareTo(selfStampUuid) == 0) {
219223
// clear the lock if locked and matches uuid
220224
aContext.ensureActive().clear(fileLockKey);
221225
closingContext = aContext;
222226
logSelf(isRecovery ? "FileLock: Cleared in Recovery path" : "FileLock: Cleared");
223-
} else if (! isRecovery) {
227+
} else if (!isRecovery) {
224228
throw new AlreadyClosedException("FileLock: Expected to be locked during close.This=" + this + " existingUuid=" + existingUuid); // The string append methods should handle null arguments.
225229
}
226-
}
227-
});
230+
});
228231

229-
if (agilityContext.isClosed()) {
230-
// Here: this is considered to be a recovery path, may bypass closed context.
231-
agilityContext.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_FILE_LOCK_CLEAR,
232-
agilityContext.applyInRecoveryPath(fileLockFunc));
233-
} else {
234-
// Here: this called during directory close to ensure cleared lock -
235-
agilityContext.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_FILE_LOCK_CLEAR,
236-
agilityContext.apply(fileLockFunc));
237-
}
238-
agilityContext.recordEvent(LuceneEvents.Events.LUCENE_FILE_LOCK_DURATION, System.nanoTime() - lockStartTime);
239-
boolean flushed = false;
240-
try {
241-
closed = true; // prevent lock stamp update
242-
agilityContext.flush();
243-
flushed = true;
244-
} finally {
245-
closed = flushed; // allow close retry
246-
closingContext = null;
232+
if (agilityContext.isClosed()) {
233+
// Here: this is considered to be a recovery path, may bypass closed context.
234+
agilityContext.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_FILE_LOCK_CLEAR,
235+
agilityContext.applyInRecoveryPath(fileLockFunc));
236+
} else {
237+
// Here: this called during directory close to ensure cleared lock -
238+
agilityContext.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_FILE_LOCK_CLEAR,
239+
agilityContext.apply(fileLockFunc));
240+
}
241+
agilityContext.recordEvent(LuceneEvents.Events.LUCENE_FILE_LOCK_DURATION, System.nanoTime() - lockStartTime);
242+
boolean flushed = false;
243+
try {
244+
closed = true; // prevent lock stamp update
245+
agilityContext.flush();
246+
flushed = true;
247+
} finally {
248+
closed = flushed; // allow close retry
249+
closingContext = null;
250+
}
247251
}
248252
}
249253

0 commit comments

Comments
 (0)