From 416419e4c9e453dbe03071b6f1f5d1c756341a17 Mon Sep 17 00:00:00 2001
From: dengweisysu <wesleydeng@tencent.com>
Date: Tue, 3 Sep 2019 02:56:22 +0800
Subject: [PATCH] Sync translog without lock when trim unreferenced readers
 (#46203)

With this change, we can avoid blocking writing threads when trimming
unreferenced readers; hence improving the translog writing performance
in async durability mode.

Close #46201
---
 .../index/translog/Translog.java              | 29 +++++++++++++------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/translog/Translog.java b/server/src/main/java/org/elasticsearch/index/translog/Translog.java
index 5f7f834c5e2..8df550f613e 100644
--- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java
+++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java
@@ -1678,6 +1678,7 @@ 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
@@ -1691,22 +1692,14 @@ 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 [" +
@@ -1715,6 +1708,24 @@ 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;
+            }
+        }
     }
 
     /**