Introduce retention lease state file #39004

jasontedor · 2019-02-16T20:15:27Z

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

Relates #37165
Closes #38588
Closes #39032

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

elasticmachine · 2019-02-16T20:15:29Z

Pinging @elastic/es-distributed

* master: Integrate retention leases to recovery from remote (elastic#38829)

jasontedor · 2019-02-16T22:19:24Z

@elasticmachine run elasticsearch-ci/oss-distro-docs

jasontedor · 2019-02-16T23:42:37Z

.../multi-cluster/src/test/java/org/elasticsearch/xpack/indexlifecycle/CCRIndexLifecycleIT.java

@@ -278,7 +278,6 @@ public void testCcrAndIlmWithRollover() throws Exception {
        }
    }

-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/37165")


The reason that we can remove this awaits fix is because the test is asserting that the shrink action is performed on the follower. The shrink action is prevented from being performed on the follower is any shard history retention leases exist on the follower with source ccr. The reason that these leases exist on the follower is because we copied them over from the commit point during recovery from remote! With this PR, this will no longer happen and this test can pass. 😌

DaveCTurner

This has helped me with my flushing woes greatly. I left a couple of questions about tests.

DaveCTurner · 2019-02-18T13:58:06Z

server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java

@@ -304,76 +289,6 @@ public void testRetentionLeaseStats() throws IOException {
        }
    }

-    public void testRecoverFromStoreReserveRetentionLeases() throws Exception {


This test seems to assert something useful, namely that reinitShard() gets the same retention leases back again. Can we keep it?

Note that reinitShard is a test-only method. However, I agree it's useful to have a test that we load the expected retention leases when we recover from store, even if we are not storing them in the commit point any longer.

Note that testPersistence already tests this. I think that we can remove this test.

+1 to remove it.

DaveCTurner · 2019-02-18T14:29:08Z

server/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java

@@ -307,6 +307,7 @@ public void finalizeRecovery(final long globalCheckpoint, ActionListener<Void> l
            indexShard.updateGlobalCheckpointOnReplica(globalCheckpoint, "finalizing recovery");
            // Persist the global checkpoint.
            indexShard.sync();
+            indexShard.persistRetentionLeases();


Do we have a test that shows that we need to do this here? I deleted this line and :server:unittest and :server:integtest both still passed (once). Is this expected?

I pushed 162cf77.

…ate-file * elastic/master: Remove tests and branches that will never execute (elastic#38772) also check ccr stats api return empty response in ensureNoCcrTasks() Add overlapping, before, after filters to intervals query (elastic#38999) Mute test elastic#38949 Add remote recovery to ShardFollowTaskReplicationTests (elastic#39007) [ML] More advanced post-test cleanup of ML indices (elastic#39049) wait for shard to be allocated before executing a resume follow api Update track-total-hits.asciidoc Force kill testcluster nodes (elastic#37353) Make pullFixture a task dependency of resolveAllDependencies (elastic#38956) set minimum supported version (elastic#39043) Enforce Completion Context Limit (elastic#38675) Mute test Don't close caches while there might still be in-flight requests. (elastic#38958) Fix elastic#38623 remove xpack namespace REST API (elastic#38625) Add data frame feature (elastic#38934) Test bi-directional index following during a rolling upgrade. (elastic#38962) Generate mvn pom for ssl-config library (elastic#39019) Mute testRetentionLeaseIsRenewedDuringRecovery

jasontedor · 2019-02-18T16:19:10Z

@DaveCTurner Thanks for looking. I have responded to your feedback.

dnhatn

I left some comments but this is really good already.

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

server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java

server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java

server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseBackgroundSyncAction.java

jasontedor · 2019-02-18T16:58:40Z

Thanks @dnhatn, I have responded to your feedback.

dnhatn

I left a small comment.

server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java

dnhatn

🎉

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

Introduce retention lease state file

da6ca36

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

jasontedor added >enhancement v7.0.0 :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. v6.7.0 v8.0.0 v7.2.0 labels Feb 16, 2019

jasontedor requested review from bleskes, ywelsch and dnhatn February 16, 2019 20:15

jasontedor mentioned this pull request Feb 16, 2019

Shard history retention leases #37165

Closed

24 tasks

jasontedor added 5 commits February 16, 2019 15:17

Remove dead field

2aa63fd

Add Javadocs

0fe1cbc

Merge branch 'master' into retention-lease-state-file

509c575

* master: Integrate retention leases to recovery from remote (elastic#38829)

Remove awaits fix

0465ded

Remove imports

2ff5b27

jasontedor requested a review from DaveCTurner February 16, 2019 22:01

jasontedor added 4 commits February 16, 2019 17:04

Fix tests

64a8cd4

Tweak message

0fb1a72

Add clarifying comment

f906152

Add test for ordering

542bad6

jasontedor added 5 commits February 16, 2019 17:19

Fix docs tests

aca06cd

Fix checkstyle

b73fcf0

Add one more test

bf454a3

Fix test

9168a60

Add additional test for duplicates

a66c097

jasontedor commented Feb 16, 2019

View reviewed changes

Remove awaits fix

539a982

jasontedor added 3 commits February 16, 2019 18:44

Fix imports

4d9efa3

Refactor loading

f1c8ebc

Correct Javadoc

9739244

jasontedor mentioned this pull request Feb 18, 2019

DocsClientYamlTestSuiteIT {yaml=reference/indices/flush/line_77} failures #39032

Closed

DaveCTurner reviewed Feb 18, 2019

View reviewed changes

jasontedor added 3 commits February 18, 2019 11:01

Add test for persistence after recovery

162cf77

Remove unnecessary line wrap

ee4d548

jasontedor requested a review from DaveCTurner February 18, 2019 16:19

dnhatn reviewed Feb 18, 2019

View reviewed changes

jasontedor added 3 commits February 18, 2019 11:40

Remove unneeded restriction

1675507

Dedicated lock, and add test

0762eda

Remove indirection in soft deletes policy

5d727ab

jasontedor requested a review from dnhatn February 18, 2019 16:58

dnhatn reviewed Feb 18, 2019

View reviewed changes

server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java Outdated Show resolved Hide resolved

jasontedor added 2 commits February 18, 2019 12:43

Fix imports

cf247be

Ensure we always persist latest

e4dbf01

jasontedor requested a review from dnhatn February 18, 2019 20:01

dnhatn approved these changes Feb 18, 2019

View reviewed changes

jasontedor merged commit 331ef9d into elastic:master Feb 18, 2019

jasontedor added a commit that referenced this pull request Feb 18, 2019

Introduce retention lease state file (#39004)

aabde0d

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

jasontedor deleted the retention-lease-state-file branch February 18, 2019 22:13

jasontedor added a commit that referenced this pull request Feb 18, 2019

Introduce retention lease state file (#39004)

8142760

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

jasontedor added a commit that referenced this pull request Feb 18, 2019

Introduce retention lease state file (#39004)

2d8f6b6

This commit moves retention leases from being persisted in the Lucene commit point to being persisted in a dedicated state file.

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

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

Introduce retention lease state file #39004

Introduce retention lease state file #39004

jasontedor commented Feb 16, 2019 •

edited

Loading

elasticmachine commented Feb 16, 2019

jasontedor commented Feb 16, 2019

jasontedor Feb 16, 2019

DaveCTurner left a comment

DaveCTurner Feb 18, 2019

jasontedor Feb 18, 2019

jasontedor Feb 18, 2019

dnhatn Feb 18, 2019

DaveCTurner Feb 18, 2019

jasontedor Feb 18, 2019

jasontedor commented Feb 18, 2019

dnhatn left a comment

jasontedor commented Feb 18, 2019

dnhatn left a comment

dnhatn left a comment

Introduce retention lease state file #39004

Introduce retention lease state file #39004

Conversation

jasontedor commented Feb 16, 2019 • edited Loading

elasticmachine commented Feb 16, 2019

jasontedor commented Feb 16, 2019

jasontedor Feb 16, 2019

Choose a reason for hiding this comment

DaveCTurner left a comment

Choose a reason for hiding this comment

DaveCTurner Feb 18, 2019

Choose a reason for hiding this comment

jasontedor Feb 18, 2019

Choose a reason for hiding this comment

jasontedor Feb 18, 2019

Choose a reason for hiding this comment

dnhatn Feb 18, 2019

Choose a reason for hiding this comment

DaveCTurner Feb 18, 2019

Choose a reason for hiding this comment

jasontedor Feb 18, 2019

Choose a reason for hiding this comment

jasontedor commented Feb 18, 2019

dnhatn left a comment

Choose a reason for hiding this comment

jasontedor commented Feb 18, 2019

dnhatn left a comment

Choose a reason for hiding this comment

dnhatn left a comment

Choose a reason for hiding this comment

jasontedor commented Feb 16, 2019 •

edited

Loading