Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrimUnreferencedReaders: move sync translog operation outside writeLock to improve performance #46203

Merged
merged 9 commits into from
Sep 2, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -1670,6 +1670,7 @@ public void rollGeneration() throws IOException {
* required generation
*/
public void trimUnreferencedReaders() throws IOException {
List<TranslogReader> toDeleteReaderList = new ArrayList<>();
try (ReleasableLock ignored = writeLock.acquire()) {
if (closed.get()) {
// we're shutdown potentially on some tragic event, don't delete anything
Expand All @@ -1690,15 +1691,8 @@ public void trimUnreferencedReaders() throws IOException {
break;
}
iterator.remove();
toDeleteReaderList.add(reader);
IOUtils.closeWhileHandlingException(reader);
final Path translogPath = reader.path();
logger.trace("delete translog file [{}], not referenced and not current anymore", translogPath);
// The checkpoint is used when opening the translog to know which files should be recovered from.
// We now update the checkpoint to ignore the file we are going to remove.
// Note that there is a provision in recoverFromFiles to allow for the case where we synced the checkpoint
// but crashed before we could delete the file.
current.sync();
deleteReaderFiles(reader);
}
assert readers.isEmpty() == false || current.generation == minReferencedGen :
"all readers were cleaned but the minReferenceGen [" + minReferencedGen + "] is not the current writer's gen [" +
Expand All @@ -1707,6 +1701,24 @@ public void trimUnreferencedReaders() throws IOException {
closeOnTragicEvent(ex);
throw ex;
}
// Do sync outside the writeLock to avoid blocking all writing thread.
try {
dengweisysu marked this conversation as resolved.
Show resolved Hide resolved
// The checkpoint is used when opening the translog to know which files should be recovered from.
// We now update the checkpoint to ignore the file we are going to remove.
// Note that there is a provision in recoverFromFiles to allow for the case where we synced the checkpoint
// but crashed before we could delete the file.
if (!toDeleteReaderList.isEmpty()) {
sync();
dengweisysu marked this conversation as resolved.
Show resolved Hide resolved
}
for (TranslogReader reader : toDeleteReaderList) {
final Path translogPath = reader.path();
logger.trace("delete translog file [{}], not referenced and not current anymore", translogPath);
deleteReaderFiles(reader);
}
} catch (final Exception ex) {
closeOnTragicEvent(ex);
throw ex;
}
}

/**
Expand Down