Revert "Sync translog without lock when trim unreferenced readers (#46203)"

Unfortunately, with this change, we won't clean up all unreferenced
generations when reopening. We assume that there's at most one
unreferenced generation when reopening translog. The previous
implementation guarantees this assumption by syncing translog every time
after we remove a translog reader. This change, however, only syncs
translog once after we have removed all unreferenced readers (can be
more than one) and breaks the assumption.

Closes #46267

This reverts commit fd8183ee51d7cf08d9def58a2ae027714beb60de.
This commit is contained in:
Nhat Nguyen 2019-09-04 16:50:34 -04:00
parent d5ad86dad9
commit a16cb89956

View File

@ -1678,7 +1678,6 @@ public class Translog extends AbstractIndexShardComponent implements IndexShardC
* 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
@ -1692,14 +1691,22 @@ public class Translog extends AbstractIndexShardComponent implements IndexShardC
"deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation ["
+ currentFileGeneration() + "]";
for (Iterator<TranslogReader> iterator = readers.iterator(); iterator.hasNext(); ) {
TranslogReader reader = iterator.next();
if (reader.getGeneration() >= minReferencedGen) {
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 [" +
@ -1708,24 +1715,6 @@ public class Translog extends AbstractIndexShardComponent implements IndexShardC
closeOnTragicEvent(ex);
throw ex;
}
// Do sync outside the writeLock to avoid blocking all writing thread.
if (toDeleteReaderList.isEmpty() == false) {
try {
// 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.
sync();
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;
}
}
}
/**