From b9227e23c4acf2af097c8e1928ee75c825777634 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 20 Feb 2020 09:19:47 +0100 Subject: [PATCH 1/2] Separate translog deletion policy conditions --- .../index/engine/InternalEngine.java | 2 +- .../index/translog/Translog.java | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index a3f53b069e7df..0fc0e522999e2 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -539,8 +539,8 @@ public Translog.Location getTranslogLastWriteLocation() { private void revisitIndexDeletionPolicyOnTranslogSynced() throws IOException { if (combinedDeletionPolicy.hasUnreferencedCommits()) { indexWriter.deleteUnusedFiles(); - translog.trimUnreferencedReaders(); } + translog.trimUnreferencedReaders(); } @Override 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 1427b070b9d43..369eb66160767 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1646,7 +1646,24 @@ public void rollGeneration() throws IOException { * required generation */ public void trimUnreferencedReaders() throws IOException { - // move most of the data to disk to reduce the time the lock is held + // first check under read lock if any readers can be trimmed + try (ReleasableLock ignored = readLock.acquire()) { + ensureOpen(); + long minReferencedGen = Math.min( + deletionPolicy.getMinTranslogGenRequiredByLocks(), + minGenerationForSeqNo(deletionPolicy.getLocalCheckpointOfSafeCommit() + 1, current, readers)); + assert minReferencedGen >= getMinFileGeneration() : + "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] but the lowest gen available is [" + + getMinFileGeneration() + "]"; + assert minReferencedGen <= currentFileGeneration() : + "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation [" + + currentFileGeneration() + "]"; + if (minReferencedGen == getMinFileGeneration()) { + return; + } + } + + // move most of the data to disk to reduce the time the write lock is held sync(); try (ReleasableLock ignored = writeLock.acquire()) { if (closed.get()) { From 98bdba27bfa6502c3abc28c484f833028cfd67c2 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 20 Feb 2020 11:07:25 +0100 Subject: [PATCH 2/2] review comments --- .../index/translog/Translog.java | 42 +++++++++---------- 1 file changed, 20 insertions(+), 22 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 369eb66160767..f0e2b04673a71 100644 --- a/server/src/main/java/org/elasticsearch/index/translog/Translog.java +++ b/server/src/main/java/org/elasticsearch/index/translog/Translog.java @@ -1648,17 +1648,11 @@ public void rollGeneration() throws IOException { public void trimUnreferencedReaders() throws IOException { // first check under read lock if any readers can be trimmed try (ReleasableLock ignored = readLock.acquire()) { - ensureOpen(); - long minReferencedGen = Math.min( - deletionPolicy.getMinTranslogGenRequiredByLocks(), - minGenerationForSeqNo(deletionPolicy.getLocalCheckpointOfSafeCommit() + 1, current, readers)); - assert minReferencedGen >= getMinFileGeneration() : - "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] but the lowest gen available is [" - + getMinFileGeneration() + "]"; - assert minReferencedGen <= currentFileGeneration() : - "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation [" - + currentFileGeneration() + "]"; - if (minReferencedGen == getMinFileGeneration()) { + if (closed.get()) { + // we're shutdown potentially on some tragic event, don't delete anything + return; + } + if (getMinReferencedGen() == getMinFileGeneration()) { return; } } @@ -1670,17 +1664,7 @@ public void trimUnreferencedReaders() throws IOException { // we're shutdown potentially on some tragic event, don't delete anything return; } - long minReferencedGen = Math.min( - deletionPolicy.getMinTranslogGenRequiredByLocks(), - minGenerationForSeqNo(deletionPolicy.getLocalCheckpointOfSafeCommit() + 1, current, readers)); - assert minReferencedGen >= getMinFileGeneration() : - "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] but the lowest gen available is [" - + getMinFileGeneration() + "]"; - assert minReferencedGen <= currentFileGeneration() : - "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation [" - + currentFileGeneration() + "]"; - - + final long minReferencedGen = getMinReferencedGen(); for (Iterator iterator = readers.iterator(); iterator.hasNext(); ) { TranslogReader reader = iterator.next(); if (reader.getGeneration() >= minReferencedGen) { @@ -1707,6 +1691,20 @@ public void trimUnreferencedReaders() throws IOException { } } + private long getMinReferencedGen() { + assert readLock.isHeldByCurrentThread() || writeLock.isHeldByCurrentThread(); + long minReferencedGen = Math.min( + deletionPolicy.getMinTranslogGenRequiredByLocks(), + minGenerationForSeqNo(deletionPolicy.getLocalCheckpointOfSafeCommit() + 1, current, readers)); + assert minReferencedGen >= getMinFileGeneration() : + "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] but the lowest gen available is [" + + getMinFileGeneration() + "]"; + assert minReferencedGen <= currentFileGeneration() : + "deletion policy requires a minReferenceGen of [" + minReferencedGen + "] which is higher than the current generation [" + + currentFileGeneration() + "]"; + return minReferencedGen; + } + /** * deletes all files associated with a reader. package-private to be able to simulate node failures at this point */