Create missing PRRLs after primary activation #44009

DaveCTurner · 2019-07-05T12:38:45Z

Today peer recovery retention leases (PRRLs) are created when starting a
replication group from scratch and during peer recovery. However, if the
replication group was migrated from nodes running a version which does not
create PRRLs (e.g. 7.3 and earlier) then it's possible that the primary was
relocated or promoted without first establishing all the expected leases.

It's not possible to establish these leases before or during primary
activation, so we must create them as soon as possible afterwards. This gives
weaker guarantees about history retention, since there's a possibility that
history will be discarded before it can be used. In practice such situations
are expected to occur only rarely.

This commit adds the machinery to create missing leases after primary
activation, and strengthens the assertions about the existence of such leases
in order to ensure that once all the leases do exist we never again enter a
state where there's a missing lease.

Relates #41536

Today peer recovery retention leases (PRRLs) are created when starting a replication group from scratch and during peer recovery. However, if the replication group was migrated from nodes running a version which does not create PRRLs (e.g. 7.3 and earlier) then it's possible that the primary was relocated or promoted without first establishing all the expected leases. It's not possible to establish these leases before or during primary activation, so we must create them as soon as possible afterwards. This gives weaker guarantees about history retention, since there's a possibility that history will be discarded before it can be used. In practice such situations are expected to occur only rarely. This commit adds the machinery to create missing leases after primary activation, and strengthens the assertions about the existence of such leases in order to ensure that once all the leases do exist we never again enter a state where there's a missing lease. Relates elastic#41536

elasticmachine · 2019-07-05T12:38:46Z

Pinging @elastic/es-distributed

DaveCTurner · 2019-07-05T12:44:12Z

Note that this PR is against the peer-recovery-retention-leases-7.x branch rather than against peer-recovery-retention-leases. I think we do not need this change in master since a rolling upgrade to master would come from a 7.x version with PRRLs; even if we performed two rolling upgrades in such quick succession that the 7.x primary didn't have time to create any missing leases, it would still create leases for the 8.0 peers during the upgrade. I will open a PR against peer-recovery-retention-leases to strengthen the assertions in master to that effect, although we don't have any double-rolling-ugprade tests today so I am not sure how meaningful this will be.

DaveCTurner · 2019-07-05T13:13:25Z

@elasticmachine please run elasticsearch-ci/2

I opened #44011 as the failure seems unrelated and occurs elsewhere.

ywelsch

I've left two minor comments, looking good o.w.
I would like for @dnhatn to have a look as well though

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

ywelsch · 2019-07-05T13:25:39Z

qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java

+            case OLD:
+                Settings.Builder settings = Settings.builder()
+                    .put(IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), between(1, 5))
+                    .put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 1)


perhaps randomly 0 or 1 replica?

++ see 1f2a197.

DaveCTurner · 2019-07-05T13:35:38Z

even if we performed two rolling upgrades in such quick succession that the 7.x primary didn't have time to create any missing leases, it would still create leases for the 8.0 peers during the upgrade

Bah this is incorrect. We know that every 8.0 peer will have a lease, but we can't be certain that every 7.x peer has one. Therefore this PR will need forward-porting to peer-recovery-retention-leases.

@ywelsch

Thanks @ywelsch, dismissing this pending @dnhatn taking a look too so I don't forget this is still pending.

dnhatn

I left a small ask but LGTM. Thanks @DaveCTurner.

dnhatn · 2019-07-08T02:06:10Z

server/src/test/java/org/elasticsearch/index/seqno/PeerRecoveryRetentionLeaseCreationIT.java

+        return false;
+    }
+
+    public void testCanRecoverFromStoreWithoutPeerRecoveryRetentionLease() throws Exception {


Can we add a full cluster restart test with 0-2 replicas then verify that after the cluster is upgraded, every copy has PRRL installed properly.

Of course, yes. I pushed aa7ac54.

…ls-bwc-create-after-primary-activation

Today peer recovery retention leases (PRRLs) are created when starting a replication group from scratch and during peer recovery. However, if the replication group was migrated from nodes running a version which does not create PRRLs (e.g. 7.3 and earlier) then it's possible that the primary was relocated or promoted without first establishing all the expected leases. It's not possible to establish these leases before or during primary activation, so we must create them as soon as possible afterwards. This gives weaker guarantees about history retention, since there's a possibility that history will be discarded before it can be used. In practice such situations are expected to occur only rarely. This commit adds the machinery to create missing leases after primary activation, and strengthens the assertions about the existence of such leases in order to ensure that once all the leases do exist we never again enter a state where there's a missing lease. Relates #41536

DaveCTurner added >enhancement :Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. labels Jul 5, 2019

DaveCTurner requested review from ywelsch and dnhatn July 5, 2019 12:38

DaveCTurner mentioned this pull request Jul 5, 2019

Retain history for peer recovery using leases #41536

Closed

10 tasks

ywelsch previously approved these changes Jul 5, 2019

View reviewed changes

Random number of replicas

1f2a197

dnhatn approved these changes Jul 8, 2019

View reviewed changes

DaveCTurner added 2 commits July 8, 2019 08:50

Merge branch 'peer-recovery-retention-leases-7.x' into 2019-07-05-prr…

65b785d

…ls-bwc-create-after-primary-activation

Show that PRRLs are created in a full-cluster restart too

aa7ac54

DaveCTurner merged commit 4b19a4b into elastic:peer-recovery-retention-leases-7.x Jul 8, 2019

DaveCTurner deleted the 2019-07-05-prrls-bwc-create-after-primary-activation branch July 8, 2019 11:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create missing PRRLs after primary activation #44009

Create missing PRRLs after primary activation #44009

DaveCTurner commented Jul 5, 2019

elasticmachine commented Jul 5, 2019

DaveCTurner commented Jul 5, 2019

DaveCTurner commented Jul 5, 2019

ywelsch left a comment

ywelsch Jul 5, 2019

DaveCTurner Jul 5, 2019

DaveCTurner commented Jul 5, 2019

dnhatn left a comment

dnhatn Jul 8, 2019

DaveCTurner Jul 8, 2019

Create missing PRRLs after primary activation #44009

Create missing PRRLs after primary activation #44009

Conversation

DaveCTurner commented Jul 5, 2019

elasticmachine commented Jul 5, 2019

DaveCTurner commented Jul 5, 2019

DaveCTurner commented Jul 5, 2019

ywelsch left a comment

Choose a reason for hiding this comment

ywelsch Jul 5, 2019

Choose a reason for hiding this comment

DaveCTurner Jul 5, 2019

Choose a reason for hiding this comment

DaveCTurner commented Jul 5, 2019

dnhatn left a comment

Choose a reason for hiding this comment

dnhatn Jul 8, 2019

Choose a reason for hiding this comment

DaveCTurner Jul 8, 2019

Choose a reason for hiding this comment