From 99862330bcc807d5442ec4b4193f40ab9bb7d0b8 Mon Sep 17 00:00:00 2001 From: wesleydeng Date: Wed, 21 Aug 2019 14:09:17 +0800 Subject: [PATCH 1/5] do sync before closeIntoReader to make sure we move most of the data to disk outside of the lock --- .../main/java/org/elasticsearch/index/translog/Translog.java | 4 ++++ 1 file changed, 4 insertions(+) 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 b98f0f2b6438d..4804c57279d32 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1641,6 +1641,10 @@ public TranslogGeneration getMinGenerationForSeqNo(final long seqNo) { * @throws IOException if an I/O exception occurred during any file operations */ public void rollGeneration() throws IOException { + //make sure we move most of the data to disk outside of the writeLock + //and then the time the lock is held can be reduced + sync(); + try (Releasable ignored = writeLock.acquire()) { try { final TranslogReader reader = current.closeIntoReader(); From 96e97417772f07db4b40b8c3c90f678339bdc416 Mon Sep 17 00:00:00 2001 From: wesleydeng Date: Wed, 21 Aug 2019 21:31:46 +0800 Subject: [PATCH 2/5] modify comments in Translog.rollGeneration --- .../main/java/org/elasticsearch/index/translog/Translog.java | 5 ++--- 1 file changed, 2 insertions(+), 3 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 4804c57279d32..2faac1cb6671d 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1641,10 +1641,9 @@ public TranslogGeneration getMinGenerationForSeqNo(final long seqNo) { * @throws IOException if an I/O exception occurred during any file operations */ public void rollGeneration() throws IOException { - //make sure we move most of the data to disk outside of the writeLock - //and then the time the lock is held can be reduced + // make sure we move most of the data to disk outside of the writeLock + // in order to reduce the time the lock is held since it's blocking all threads sync(); - try (Releasable ignored = writeLock.acquire()) { try { final TranslogReader reader = current.closeIntoReader(); From 448682af84e9821513507fef4c2207e77d72f69c Mon Sep 17 00:00:00 2001 From: wesleydeng Date: Sat, 31 Aug 2019 22:34:37 +0800 Subject: [PATCH 3/5] trimUnreferencedReaders: move sync translog operation outside writeLock --- .../index/translog/Translog.java | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 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 305fcf6268912..dd1eb4b647e25 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1670,6 +1670,7 @@ public void rollGeneration() throws IOException { * required generation */ public void trimUnreferencedReaders() throws IOException { + List toDeleteReaderList = new ArrayList<>(); try (ReleasableLock ignored = writeLock.acquire()) { if (closed.get()) { // we're shutdown potentially on some tragic event, don't delete anything @@ -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 [" + @@ -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 { + // 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(); + } + 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; + } } /** From e6744c2ca6cc0784a10f3cca3359c1c52227d6f5 Mon Sep 17 00:00:00 2001 From: wesleydeng Date: Mon, 2 Sep 2019 18:11:54 +0800 Subject: [PATCH 4/5] trimUnreferencedReaders: move try-catch inside if condition --- .../index/translog/Translog.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 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 dd1eb4b647e25..89ee29689c550 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1684,7 +1684,6 @@ public void trimUnreferencedReaders() throws IOException { "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation [" + currentFileGeneration() + "]"; - for (Iterator iterator = readers.iterator(); iterator.hasNext(); ) { TranslogReader reader = iterator.next(); if (reader.getGeneration() >= minReferencedGen) { @@ -1702,22 +1701,22 @@ public void trimUnreferencedReaders() throws IOException { throw ex; } // Do sync outside the writeLock to avoid blocking all writing thread. - 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. - if (!toDeleteReaderList.isEmpty()) { + 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; } - 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; } } From c2dbddc246bcc94fb428ba345298948a052c9c43 Mon Sep 17 00:00:00 2001 From: wesleydeng Date: Wed, 9 Oct 2019 20:03:50 +0800 Subject: [PATCH 5/5] trimUnreferencedReaders: sync before write lock --- .../index/translog/Translog.java | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 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 33065d21a8510..2fb0d0ba5278c 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1670,7 +1670,8 @@ public void rollGeneration() throws IOException { * required generation */ public void trimUnreferencedReaders() throws IOException { - List toDeleteReaderList = new ArrayList<>(); + // move most of the data to disk to reduce the time the lock is held + sync(); try (ReleasableLock ignored = writeLock.acquire()) { if (closed.get()) { // we're shutdown potentially on some tragic event, don't delete anything @@ -1684,14 +1685,23 @@ public void trimUnreferencedReaders() throws IOException { "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation [" + currentFileGeneration() + "]"; + for (Iterator 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. + // sync at once to make sure that there's at most one unreferenced generation. + 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 [" + @@ -1700,24 +1710,6 @@ public void trimUnreferencedReaders() throws IOException { 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; - } - } } /**