Recover peers from translog, ignoring soft deletes #38904

DaveCTurner · 2019-02-14T15:33:29Z

Today if soft deletes are enabled then we read the operations needed for peer
recovery from Lucene. However we do not currently make any attempt to retain
history in Lucene specifically for peer recoveries so we may discard it and
fall back to a more expensive file-based recovery. Yet we still retain
sufficient history in the translog to perform an operations-based peer
recovery.

In the long run we would like to fix this by retaining more history in Lucene,
possibly using shard history retention leases (#37165). For now, however, this
commit reverts to performing peer recoveries using the history retained in the
translog regardless of whether soft deletes are enabled or not.

Today if soft deletes are enabled then we read the operations needed for peer recovery from Lucene. However we do not currently make any attempt to retain history in Lucene specifically for peer recoveries so we may discard it and fall back to a more expensive file-based recovery. Yet we still retain sufficient history in the translog to perform an operations-based peer recovery. In the long run we would like to fix this by retaining more history in Lucene, possibly using shard history retention leases (elastic#37165). For now, however, this commit reverts to performing peer recoveries using the history retained in the translog regardless of whether soft deletes are enabled or not.

elasticmachine · 2019-02-14T15:33:31Z

Pinging @elastic/es-distributed

dnhatn

I left some comments but LGTM. You need to remove or mute testRecoveryWithOutOfOrderDeleteWithSoftDeletes.

dnhatn · 2019-02-14T15:46:13Z

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

-            return softDeletesPolicy.acquireRetentionLock();
+            final Closeable softDeletesRetentionLock = softDeletesPolicy.acquireRetentionLock();
+            final Closeable translogRetentionLock = translog.acquireRetentionLock();
+            return () -> {


maybe use () -> IOUtils.close(softDeletesRetentionLock, translogRetentionLock) so we always close both of them.

dnhatn · 2019-02-14T15:52:43Z

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

@@ -2575,7 +2566,12 @@ public final long getMinRetainedSeqNo() {
    @Override
    public Closeable acquireRetentionLock() {
        if (softDeleteEnabled) {
-            return softDeletesPolicy.acquireRetentionLock();
+            final Closeable softDeletesRetentionLock = softDeletesPolicy.acquireRetentionLock();
+            final Closeable translogRetentionLock = translog.acquireRetentionLock();


I think we should release softDeletesRetentionLock if we hit an exception while acquiring translog lock.

Good point, and similarly on release. I pushed 7b6c2a0.

dnhatn · 2019-02-14T15:58:57Z

server/src/test/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java

@@ -786,4 +788,55 @@ public void sendRequest(Transport.Connection connection, long requestId, String
            assertHitCount(client().prepareSearch(indexName).get(), numDocs);
        }
    }
+
+    @TestLogging("org.elasticsearch.indices.recovery:TRACE")
+    public void testHistoryRetention() throws Exception {


Is this test a left-over?

No, this test motivates this change, in the sense that it fails on master but passes on this branch.

dnhatn · 2019-02-14T16:30:41Z

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

+            } catch (Exception e) {
+                try {
+                    softDeletesRetentionLock.close();
+                } catch (IOException ioexception) {


nit: make softDeletesRetentionLock Releasable so we won't have to catch here.

Today if soft deletes are enabled then we read the operations needed for peer recovery from Lucene. However we do not currently make any attempt to retain history in Lucene specifically for peer recoveries so we may discard it and fall back to a more expensive file-based recovery. Yet we still retain sufficient history in the translog to perform an operations-based peer recovery. In the long run we would like to fix this by retaining more history in Lucene, possibly using shard history retention leases (#37165). For now, however, this commit reverts to performing peer recoveries using the history retained in the translog regardless of whether soft deletes are enabled or not.

* elastic/master: Avoid double term construction in DfsPhase (elastic#38716) Fix typo in DateRange docs (yyy → yyyy) (elastic#38883) Introduced class reuses follow parameter code between ShardFollowTasks (elastic#38910) Ensure random timestamps are within search boundary (elastic#38753) [CI] Muting method testFollowIndex in IndexFollowingIT Update Lucene snapshot repo for 7.0.0-beta1 (elastic#38946) SQL: Doc on syntax (identifiers in particular) (elastic#38662) Upgrade to Gradle 5.2.1 (elastic#38880) Tie break search shard iterator comparisons on cluster alias (elastic#38853) Also mmap cfs files for hybridfs (elastic#38940) Build: Fix issue with test status logging (elastic#38799) Adapt FullClusterRestartIT on master (elastic#38856) Fix testAutoFollowing test to use createLeaderIndex() helper method. Migrate muted auto follow rolling upgrade test and unmute this test (elastic#38900) ShardBulkAction ignore primary response on primary (elastic#38901) Recover peers from translog, ignoring soft deletes (elastic#38904) Fix NPE on Stale Index in IndicesService (elastic#38891) Smarter CCR concurrent file chunk fetching (elastic#38841) Fix intermittent failure in ApiKeyIntegTests (elastic#38627) re-enable SmokeTestWatcherWithSecurityIT (elastic#38814)

Currently, we ignore soft-deletes in peer recovery, thus estimateNumberOfHistoryOperations should always use translog. Relates #38904

Currently, we ignore soft-deletes in peer recovery, thus estimateNumberOfHistoryOperations should always use translog. Relates elastic#38904

Thanks to peer recovery retention leases we now retain the history needed to perform peer recoveries from the index instead of from the translog. This commit adjusts the peer recovery process to do so, and also adjusts it to use the existence of a retention lease to decide whether or not to attempt an operations-based recovery. Reverts elastic#38904 and elastic#42211 Relates elastic#41536

Thanks to peer recovery retention leases we now retain the history needed to perform peer recoveries from the index instead of from the translog. This commit adjusts the peer recovery process to do so, and also adjusts it to use the existence of a retention lease to decide whether or not to attempt an operations-based recovery. Reverts #38904 and #42211 Relates #41536

Today we recover a replica by copying operations from the primary's translog. However we also retain some historical operations in the index itself, as long as soft-deletes are enabled. This commit adjusts peer recovery to use the operations in the index for recovery rather than those in the translog, and ensures that the replication group retains enough history for use in peer recovery by means of retention leases. Reverts #38904 and #42211 Relates #41536 Backport of #45136 to 7.x.

DaveCTurner added >bug :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v7.0.0 v8.0.0 v7.2.0 labels Feb 14, 2019

DaveCTurner requested review from ywelsch and dnhatn February 14, 2019 15:33

dnhatn approved these changes Feb 14, 2019

View reviewed changes

DaveCTurner added 2 commits February 14, 2019 16:14

Deal with exceptions when obtaining both retention locks

7b6c2a0

IOUtils.close

53d63f6

dnhatn reviewed Feb 14, 2019

View reviewed changes

Neater to be Releasable

755ce5a

DaveCTurner merged commit 3f1125f into elastic:master Feb 15, 2019

DaveCTurner deleted the 2019-02-14-translog-based-peer-recovery branch February 15, 2019 08:43

DaveCTurner mentioned this pull request Feb 16, 2019

[CI] FollowerFailOverIT testFailOverOnFollower fails frequently #38633

Closed

dnhatn mentioned this pull request Feb 22, 2019

FsBlobStoreRepositoryIT#testSnapshotRestore fails reproducibly #39299

Closed

jakelandis added v7.0.0-rc2 and removed v7.0.0 labels Apr 3, 2019

dnhatn mentioned this pull request May 20, 2019

Use translog to estimate number of operations in recovery #42211

Merged

dnhatn added a commit that referenced this pull request May 21, 2019

Estimate num history ops should always use translog (#42211)

c1aef4b

Currently, we ignore soft-deletes in peer recovery, thus estimateNumberOfHistoryOperations should always use translog. Relates #38904

dnhatn added a commit that referenced this pull request May 22, 2019

Estimate num history ops should always use translog (#42211)

4d55e9e

Currently, we ignore soft-deletes in peer recovery, thus estimateNumberOfHistoryOperations should always use translog. Relates #38904

DaveCTurner mentioned this pull request Jun 11, 2019

Compact index when closing #42445

Closed

DaveCTurner mentioned this pull request Jul 25, 2019

Recover peers using history from Lucene #44853

Merged

This was referenced Aug 2, 2019

Use index for peer recovery instead of translog #45136

Merged

Use index for peer recovery instead of translog #45137

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Recover peers from translog, ignoring soft deletes #38904

Recover peers from translog, ignoring soft deletes #38904

DaveCTurner commented Feb 14, 2019

elasticmachine commented Feb 14, 2019

dnhatn left a comment

dnhatn Feb 14, 2019

dnhatn Feb 14, 2019

DaveCTurner Feb 14, 2019

dnhatn Feb 14, 2019

DaveCTurner Feb 14, 2019

dnhatn Feb 14, 2019

Recover peers from translog, ignoring soft deletes #38904

Recover peers from translog, ignoring soft deletes #38904

Conversation

DaveCTurner commented Feb 14, 2019

elasticmachine commented Feb 14, 2019

dnhatn left a comment

Choose a reason for hiding this comment

dnhatn Feb 14, 2019

Choose a reason for hiding this comment

dnhatn Feb 14, 2019

Choose a reason for hiding this comment

DaveCTurner Feb 14, 2019

Choose a reason for hiding this comment

dnhatn Feb 14, 2019

Choose a reason for hiding this comment

DaveCTurner Feb 14, 2019

Choose a reason for hiding this comment

dnhatn Feb 14, 2019

Choose a reason for hiding this comment