From 98f423976ecb21ecc08d90c687f1b5890057ba72 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 28 Jan 2019 22:31:28 -0500 Subject: [PATCH 01/26] Introduce retention leases versioning Because concurrent sync requests from a primary to its replicas could be in flight, it can be the case that an older retention leases collection arrives and is processed on the replica after a newer retention leases collection has arrived and been processed. Without a defense, in this case the replica would overwrite the newer retention leases with the older retention leases. This commit addresses this issue by introducing a versioning scheme to retention leases. This versioning scheme is used to resolve out-of-order processing on the replica. We persist this version into Lucene and restore it on recovery. The encoding of retention leases is starting to get a little ugly. We can consider addressing this in a follow-up. --- .../index/engine/EngineConfig.java | 9 +- .../index/engine/InternalEngine.java | 3 +- .../index/engine/SoftDeletesPolicy.java | 11 +- .../index/seqno/ReplicationTracker.java | 28 +++-- .../index/seqno/RetentionLease.java | 34 ++++-- .../index/seqno/RetentionLeaseSyncAction.java | 13 +- .../index/seqno/RetentionLeaseSyncer.java | 4 +- .../index/seqno/RetentionLeases.java | 49 ++++++++ .../elasticsearch/index/shard/IndexShard.java | 12 +- .../engine/CombinedDeletionPolicyTests.java | 12 +- .../index/engine/InternalEngineTests.java | 41 +++++-- .../index/engine/SoftDeletesPolicyTests.java | 10 +- ...ReplicationTrackerRetentionLeaseTests.java | 111 ++++++++++++++---- .../index/seqno/ReplicationTrackerTests.java | 2 +- .../seqno/RetentionLeaseSyncActionTests.java | 16 +-- .../index/seqno/RetentionLeaseSyncIT.java | 26 ++-- .../index/seqno/RetentionLeaseTests.java | 11 +- .../shard/IndexShardRetentionLeaseTests.java | 64 ++++++---- .../index/shard/RefreshListenersTests.java | 31 ++++- .../index/engine/EngineTestCase.java | 13 +- .../index/engine/FollowingEngineTests.java | 3 +- 21 files changed, 347 insertions(+), 156 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java diff --git a/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java b/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java index 1cc92319b5e45..7716cf93ffd6b 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java +++ b/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java @@ -35,7 +35,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.codec.CodecService; import org.elasticsearch.index.mapper.ParsedDocument; -import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.store.Store; import org.elasticsearch.index.translog.TranslogConfig; @@ -43,7 +43,6 @@ import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.threadpool.ThreadPool; -import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.function.LongSupplier; @@ -81,7 +80,7 @@ public final class EngineConfig { @Nullable private final CircuitBreakerService circuitBreakerService; private final LongSupplier globalCheckpointSupplier; - private final Supplier> retentionLeasesSupplier; + private final Supplier retentionLeasesSupplier; /** * A supplier of the outstanding retention leases. This is used during merged operations to determine which operations that have been @@ -89,7 +88,7 @@ public final class EngineConfig { * * @return a supplier of outstanding retention leases */ - public Supplier> retentionLeasesSupplier() { + public Supplier retentionLeasesSupplier() { return retentionLeasesSupplier; } @@ -141,7 +140,7 @@ public EngineConfig(ShardId shardId, String allocationId, ThreadPool threadPool, List externalRefreshListener, List internalRefreshListener, Sort indexSort, CircuitBreakerService circuitBreakerService, LongSupplier globalCheckpointSupplier, - Supplier> retentionLeasesSupplier, + Supplier retentionLeasesSupplier, LongSupplier primaryTermSupplier, TombstoneDocSupplier tombstoneDocSupplier) { this.shardId = shardId; 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 df52c6bc0213f..eda33c9698432 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -76,6 +76,7 @@ import org.elasticsearch.index.merge.OnGoingMerge; import org.elasticsearch.index.seqno.LocalCheckpointTracker; import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.shard.ElasticsearchMergePolicy; @@ -2342,7 +2343,7 @@ protected void commitIndexWriter(final IndexWriter writer, final Translog transl * We sample these from the policy (which occurs under a lock) to ensure that we have a consistent view of the minimum * retained sequence number, and the retention leases. */ - final Tuple> retentionPolicy = softDeletesPolicy.getRetentionPolicy(); + final Tuple retentionPolicy = softDeletesPolicy.getRetentionPolicy(); commitData.put(Engine.MIN_RETAINED_SEQNO, Long.toString(retentionPolicy.v1())); commitData.put(Engine.RETENTION_LEASES, RetentionLease.encodeRetentionLeases(retentionPolicy.v2())); } diff --git a/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java b/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java index a2452d4b53eb9..da3194620bbf8 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java +++ b/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java @@ -25,10 +25,10 @@ import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.index.mapper.SeqNoFieldMapper; import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.translog.Translog; -import java.util.Collection; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.LongSupplier; @@ -46,15 +46,15 @@ final class SoftDeletesPolicy { private long retentionOperations; // The min seq_no value that is retained - ops after this seq# should exist in the Lucene index. private long minRetainedSeqNo; - private Collection retentionLeases; + private RetentionLeases retentionLeases; // provides the retention leases used to calculate the minimum sequence number to retain - private final Supplier> retentionLeasesSupplier; + private final Supplier retentionLeasesSupplier; SoftDeletesPolicy( final LongSupplier globalCheckpointSupplier, final long minRetainedSeqNo, final long retentionOperations, - final Supplier> retentionLeasesSupplier) { + final Supplier retentionLeasesSupplier) { this.globalCheckpointSupplier = globalCheckpointSupplier; this.retentionOperations = retentionOperations; this.minRetainedSeqNo = minRetainedSeqNo; @@ -112,7 +112,7 @@ synchronized long getMinRetainedSeqNo() { return getRetentionPolicy().v1(); } - public synchronized Tuple> getRetentionPolicy() { + public synchronized Tuple getRetentionPolicy() { // do not advance if the retention lock is held if (retentionLockCount == 0) { /* @@ -128,6 +128,7 @@ public synchronized Tuple> getRetentionPolicy() // calculate the minimum sequence number to retain based on retention leases retentionLeases = retentionLeasesSupplier.get(); final long minimumRetainingSequenceNumber = retentionLeases + .retentionLeases() .stream() .mapToLong(RetentionLease::retainingSequenceNumber) .min() diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index 4a614d8874aff..1f88cf3f722f4 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -150,7 +150,7 @@ public class ReplicationTracker extends AbstractIndexShardComponent implements L * A callback when a new retention lease is created or an existing retention lease expires. In practice, this callback invokes the * retention lease sync action, to sync retention leases to replicas. */ - private final BiConsumer, ActionListener> onSyncRetentionLeases; + private final BiConsumer> onSyncRetentionLeases; /** * This set contains allocation IDs for which there is a thread actively waiting for the local checkpoint to advance to at least the @@ -163,11 +163,12 @@ public class ReplicationTracker extends AbstractIndexShardComponent implements L */ volatile ReplicationGroup replicationGroup; + private volatile long version = 0; private final Map retentionLeases = new HashMap<>(); - private Collection copyRetentionLeases() { + private RetentionLeases copyRetentionLeases() { assert Thread.holdsLock(this); - return Collections.unmodifiableCollection(new ArrayList<>(retentionLeases.values())); + return new RetentionLeases(version, Collections.unmodifiableCollection(new ArrayList<>(retentionLeases.values()))); } /** @@ -176,9 +177,9 @@ private Collection copyRetentionLeases() { * * @return the retention leases */ - public Collection getRetentionLeases() { + public RetentionLeases getRetentionLeases() { final boolean wasPrimaryMode; - final Collection nonExpiredRetentionLeases; + final RetentionLeases nonExpiredRetentionLeases; synchronized (this) { if (primaryMode) { // the primary calculates the non-expired retention leases and syncs them to replicas @@ -197,6 +198,7 @@ public Collection getRetentionLeases() { for (final RetentionLease expiredRetentionLease : expiredRetentionLeases) { retentionLeases.remove(expiredRetentionLease.id()); } + version++; } /* * At this point, we were either in primary mode and have updated the non-expired retention leases into the tracking map, or @@ -229,7 +231,7 @@ public RetentionLease addRetentionLease( final ActionListener listener) { Objects.requireNonNull(listener); final RetentionLease retentionLease; - final Collection currentRetentionLeases; + final RetentionLeases currentRetentionLeases; synchronized (this) { assert primaryMode; if (retentionLeases.containsKey(id)) { @@ -237,6 +239,7 @@ public RetentionLease addRetentionLease( } retentionLease = new RetentionLease(id, retainingSequenceNumber, currentTimeMillisSupplier.getAsLong(), source); retentionLeases.put(id, retentionLease); + version++; currentRetentionLeases = copyRetentionLeases(); } onSyncRetentionLeases.accept(currentRetentionLeases, listener); @@ -260,6 +263,7 @@ public synchronized RetentionLease renewRetentionLease(final String id, final lo final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, currentTimeMillisSupplier.getAsLong(), source); final RetentionLease existingRetentionLease = retentionLeases.put(id, retentionLease); + version++; assert existingRetentionLease != null; assert existingRetentionLease.retainingSequenceNumber() <= retentionLease.retainingSequenceNumber() : "retention lease renewal for [" + id + "]" @@ -274,10 +278,14 @@ public synchronized RetentionLease renewRetentionLease(final String id, final lo * * @param retentionLeases the retention leases */ - public synchronized void updateRetentionLeasesOnReplica(final Collection retentionLeases) { + public synchronized void updateRetentionLeasesOnReplica(final RetentionLeases retentionLeases) { assert primaryMode == false; - this.retentionLeases.clear(); - this.retentionLeases.putAll(retentionLeases.stream().collect(Collectors.toMap(RetentionLease::id, Function.identity()))); + if (retentionLeases.version() > version) { + this.retentionLeases.clear(); + this.retentionLeases.putAll( + retentionLeases.retentionLeases().stream().collect(Collectors.toMap(RetentionLease::id, Function.identity()))); + this.version = retentionLeases.version(); + } } public static class CheckpointState implements Writeable { @@ -537,7 +545,7 @@ public ReplicationTracker( final long globalCheckpoint, final LongConsumer onGlobalCheckpointUpdated, final LongSupplier currentTimeMillisSupplier, - final BiConsumer, ActionListener> onSyncRetentionLeases) { + final BiConsumer> onSyncRetentionLeases) { super(shardId, indexSettings); assert globalCheckpoint >= SequenceNumbers.UNASSIGNED_SEQ_NO : "illegal initial global checkpoint: " + globalCheckpoint; this.shardAllocationId = allocationId; diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java index 13e3381b11553..b8626793f08ca 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java @@ -173,9 +173,13 @@ static String encodeRetentionLease(final RetentionLease retentionLease) { * @param retentionLeases the retention leases * @return the encoding of the retention leases */ - public static String encodeRetentionLeases(final Collection retentionLeases) { + public static String encodeRetentionLeases(final RetentionLeases retentionLeases) { Objects.requireNonNull(retentionLeases); - return retentionLeases.stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(",")); + return String.format( + Locale.ROOT, + "version:%d;%s", + retentionLeases.version(), + retentionLeases.retentionLeases().stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(","))); } /** @@ -200,20 +204,32 @@ static RetentionLease decodeRetentionLease(final String encodedRetentionLease) { } /** - * Decodes retention leases encoded by {@link #encodeRetentionLeases(Collection)}. + * Decodes retention leases encoded by {@link #encodeRetentionLeases(RetentionLeases)}. * * @param encodedRetentionLeases an encoded collection of retention leases * @return the decoded retention leases */ - public static Collection decodeRetentionLeases(final String encodedRetentionLeases) { + public static RetentionLeases decodeRetentionLeases(final String encodedRetentionLeases) { Objects.requireNonNull(encodedRetentionLeases); if (encodedRetentionLeases.isEmpty()) { - return Collections.emptyList(); + return RetentionLeases.EMPTY; } - assert Arrays.stream(encodedRetentionLeases.split(",")) - .allMatch(s -> s.matches("id:[^:;,]+;retaining_seq_no:\\d+;timestamp:\\d+;source:[^:;,]+")) - : encodedRetentionLeases; - return Arrays.stream(encodedRetentionLeases.split(",")).map(RetentionLease::decodeRetentionLease).collect(Collectors.toList()); + assert encodedRetentionLeases.matches("version:\\d+;.*") : encodedRetentionLeases; + final int firstSemicolon = encodedRetentionLeases.indexOf(";"); + final long version = Long.parseLong(encodedRetentionLeases.substring("version:".length(), firstSemicolon)); + final Collection retentionLeases; + if (firstSemicolon + 1 == encodedRetentionLeases.length()) { + retentionLeases = Collections.emptyList(); + } else { + assert Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) + .allMatch(s -> s.matches("id:[^:;,]+;retaining_seq_no:\\d+;timestamp:\\d+;source:[^:;,]+")) + : encodedRetentionLeases; + retentionLeases = Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) + .map(RetentionLease::decodeRetentionLease) + .collect(Collectors.toList()); + } + + return new RetentionLeases(version, retentionLeases); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncAction.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncAction.java index 3b7df41f72d05..89a679abea591 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncAction.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncAction.java @@ -47,7 +47,6 @@ import org.elasticsearch.transport.TransportService; import java.io.IOException; -import java.util.Collection; import java.util.Objects; /** @@ -99,7 +98,7 @@ public RetentionLeaseSyncAction( */ public void syncRetentionLeasesForShard( final ShardId shardId, - final Collection retentionLeases, + final RetentionLeases retentionLeases, final ActionListener listener) { Objects.requireNonNull(shardId); Objects.requireNonNull(retentionLeases); @@ -149,9 +148,9 @@ private void flush(final IndexShard indexShard) { public static final class Request extends ReplicatedWriteRequest { - private Collection retentionLeases; + private RetentionLeases retentionLeases; - public Collection getRetentionLeases() { + public RetentionLeases getRetentionLeases() { return retentionLeases; } @@ -159,7 +158,7 @@ public Request() { } - public Request(final ShardId shardId, final Collection retentionLeases) { + public Request(final ShardId shardId, final RetentionLeases retentionLeases) { super(Objects.requireNonNull(shardId)); this.retentionLeases = Objects.requireNonNull(retentionLeases); } @@ -167,13 +166,13 @@ public Request(final ShardId shardId, final Collection retention @Override public void readFrom(final StreamInput in) throws IOException { super.readFrom(in); - retentionLeases = in.readList(RetentionLease::new); + retentionLeases = new RetentionLeases(in); } @Override public void writeTo(final StreamOutput out) throws IOException { super.writeTo(Objects.requireNonNull(out)); - out.writeCollection(retentionLeases); + retentionLeases.writeTo(out); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncer.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncer.java index 1e276eb98adaf..a19700a94da4b 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncer.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncer.java @@ -23,8 +23,6 @@ import org.elasticsearch.action.support.replication.ReplicationResponse; import org.elasticsearch.index.shard.ShardId; -import java.util.Collection; - /** * A functional interface that represents a method for syncing retention leases to replica shards after a new retention lease is added on * the primary. @@ -42,7 +40,7 @@ public interface RetentionLeaseSyncer { */ void syncRetentionLeasesForShard( ShardId shardId, - Collection retentionLeases, + RetentionLeases retentionLeases, ActionListener listener); } diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java new file mode 100644 index 0000000000000..77876e11f5755 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -0,0 +1,49 @@ +package org.elasticsearch.index.seqno; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; + +public class RetentionLeases implements Writeable { + + private final long version; + + public long version() { + return version; + } + + private final Collection retentionLeases; + + public Collection retentionLeases() { + return retentionLeases; + } + + public static RetentionLeases EMPTY = new RetentionLeases(0, Collections.emptyList()); + + public RetentionLeases(final long version, final Collection retentionLeases) { + if (version < 0) { + throw new IllegalArgumentException("version must be non-negative but was [" + version + "]"); + } + Objects.requireNonNull(retentionLeases); + this.version = version; + this.retentionLeases = Collections.unmodifiableCollection(new ArrayList<>(retentionLeases)); + } + + public RetentionLeases(final StreamInput in) throws IOException { + version = in.readVLong(); + retentionLeases = in.readList(RetentionLease::new); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + out.writeVLong(version); + out.writeCollection(retentionLeases); + } + +} diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index dc43d42c94a5c..5e88d09e2f040 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -108,6 +108,7 @@ import org.elasticsearch.index.search.stats.ShardSearchStats; import org.elasticsearch.index.seqno.ReplicationTracker; import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.shard.PrimaryReplicaSyncer.ResyncTask; @@ -142,7 +143,6 @@ import java.nio.channels.ClosedByInterruptException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.List; @@ -267,7 +267,7 @@ public IndexShard( final List searchOperationListener, final List listeners, final Runnable globalCheckpointSyncer, - final BiConsumer, ActionListener> retentionLeaseSyncer, + final BiConsumer> retentionLeaseSyncer, final CircuitBreakerService circuitBreakerService) throws IOException { super(shardRouting.shardId(), indexSettings); assert shardRouting.initializing(); @@ -1442,10 +1442,10 @@ private void innerOpenEngineAndTranslog() throws IOException { assert recoveryState.getStage() == RecoveryState.Stage.TRANSLOG : "TRANSLOG stage expected but was: " + recoveryState.getStage(); } - static Collection getRetentionLeases(final SegmentInfos segmentInfos) { + static RetentionLeases getRetentionLeases(final SegmentInfos segmentInfos) { final String committedRetentionLeases = segmentInfos.getUserData().get(Engine.RETENTION_LEASES); if (committedRetentionLeases == null) { - return Collections.emptyList(); + return RetentionLeases.EMPTY; } return RetentionLease.decodeRetentionLeases(committedRetentionLeases); } @@ -1890,7 +1890,7 @@ public void addGlobalCheckpointListener( * * @return the retention leases */ - public Collection getRetentionLeases() { + public RetentionLeases getRetentionLeases() { verifyNotClosed(); return replicationTracker.getRetentionLeases(); } @@ -1936,7 +1936,7 @@ public RetentionLease renewRetentionLease(final String id, final long retainingS * * @param retentionLeases the retention leases */ - public void updateRetentionLeasesOnReplica(final Collection retentionLeases) { + public void updateRetentionLeasesOnReplica(final RetentionLeases retentionLeases) { assert assertReplicationTarget(); verifyNotClosed(); replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); diff --git a/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java b/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java index 054bfb8bad695..617d23c44e1c6 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/CombinedDeletionPolicyTests.java @@ -22,6 +22,7 @@ import com.carrotsearch.hppc.LongArrayList; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.store.Directory; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.translog.Translog; import org.elasticsearch.index.translog.TranslogDeletionPolicy; @@ -30,7 +31,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -55,7 +55,7 @@ public void testKeepCommitsAfterGlobalCheckpoint() throws Exception { final AtomicLong globalCheckpoint = new AtomicLong(); final int extraRetainedOps = between(0, 100); final SoftDeletesPolicy softDeletesPolicy = - new SoftDeletesPolicy(globalCheckpoint::get, NO_OPS_PERFORMED, extraRetainedOps, Collections::emptyList); + new SoftDeletesPolicy(globalCheckpoint::get, NO_OPS_PERFORMED, extraRetainedOps, () -> RetentionLeases.EMPTY); TranslogDeletionPolicy translogPolicy = createTranslogDeletionPolicy(); CombinedDeletionPolicy indexPolicy = new CombinedDeletionPolicy(logger, translogPolicy, softDeletesPolicy, globalCheckpoint::get); @@ -101,7 +101,7 @@ public void testAcquireIndexCommit() throws Exception { final AtomicLong globalCheckpoint = new AtomicLong(); final int extraRetainedOps = between(0, 100); final SoftDeletesPolicy softDeletesPolicy = - new SoftDeletesPolicy(globalCheckpoint::get, -1, extraRetainedOps, Collections::emptyList); + new SoftDeletesPolicy(globalCheckpoint::get, -1, extraRetainedOps, () -> RetentionLeases.EMPTY); final UUID translogUUID = UUID.randomUUID(); TranslogDeletionPolicy translogPolicy = createTranslogDeletionPolicy(); CombinedDeletionPolicy indexPolicy = new CombinedDeletionPolicy(logger, translogPolicy, softDeletesPolicy, globalCheckpoint::get); @@ -182,7 +182,7 @@ public void testAcquireIndexCommit() throws Exception { public void testLegacyIndex() throws Exception { final AtomicLong globalCheckpoint = new AtomicLong(); - final SoftDeletesPolicy softDeletesPolicy = new SoftDeletesPolicy(globalCheckpoint::get, -1, 0, Collections::emptyList); + final SoftDeletesPolicy softDeletesPolicy = new SoftDeletesPolicy(globalCheckpoint::get, -1, 0, () -> RetentionLeases.EMPTY); final UUID translogUUID = UUID.randomUUID(); TranslogDeletionPolicy translogPolicy = createTranslogDeletionPolicy(); @@ -217,7 +217,7 @@ public void testLegacyIndex() throws Exception { public void testDeleteInvalidCommits() throws Exception { final AtomicLong globalCheckpoint = new AtomicLong(randomNonNegativeLong()); - final SoftDeletesPolicy softDeletesPolicy = new SoftDeletesPolicy(globalCheckpoint::get, -1, 0, Collections::emptyList); + final SoftDeletesPolicy softDeletesPolicy = new SoftDeletesPolicy(globalCheckpoint::get, -1, 0, () -> RetentionLeases.EMPTY); TranslogDeletionPolicy translogPolicy = createTranslogDeletionPolicy(); CombinedDeletionPolicy indexPolicy = new CombinedDeletionPolicy(logger, translogPolicy, softDeletesPolicy, globalCheckpoint::get); @@ -251,7 +251,7 @@ public void testDeleteInvalidCommits() throws Exception { public void testCheckUnreferencedCommits() throws Exception { final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.UNASSIGNED_SEQ_NO); - final SoftDeletesPolicy softDeletesPolicy = new SoftDeletesPolicy(globalCheckpoint::get, -1, 0, Collections::emptyList); + final SoftDeletesPolicy softDeletesPolicy = new SoftDeletesPolicy(globalCheckpoint::get, -1, 0, () -> RetentionLeases.EMPTY); final UUID translogUUID = UUID.randomUUID(); final TranslogDeletionPolicy translogPolicy = createTranslogDeletionPolicy(); CombinedDeletionPolicy indexPolicy = new CombinedDeletionPolicy(logger, translogPolicy, softDeletesPolicy, globalCheckpoint::get); diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index edf1925fdd798..b45589ea5c47e 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -117,6 +117,7 @@ import org.elasticsearch.index.seqno.LocalCheckpointTracker; import org.elasticsearch.index.seqno.ReplicationTracker; import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.shard.IndexSearcherWrapper; @@ -141,7 +142,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; -import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -3020,12 +3020,29 @@ public void testRecoverFromForeignTranslog() throws IOException { TranslogConfig translogConfig = new TranslogConfig(shardId, translog.location(), config.getIndexSettings(), BigArrays.NON_RECYCLING_INSTANCE); - EngineConfig brokenConfig = new EngineConfig(shardId, allocationId.getId(), - threadPool, config.getIndexSettings(), null, store, newMergePolicy(), config.getAnalyzer(), config.getSimilarity(), - new CodecService(null, logger), config.getEventListener(), IndexSearcher.getDefaultQueryCache(), - IndexSearcher.getDefaultQueryCachingPolicy(), translogConfig, TimeValue.timeValueMinutes(5), - config.getExternalRefreshListener(), config.getInternalRefreshListener(), null, - new NoneCircuitBreakerService(), () -> UNASSIGNED_SEQ_NO, Collections::emptyList, primaryTerm::get, + EngineConfig brokenConfig = new EngineConfig( + shardId, + allocationId.getId(), + threadPool, + config.getIndexSettings(), + null, + store, + newMergePolicy(), + config.getAnalyzer(), + config.getSimilarity(), + new CodecService(null, logger), + config.getEventListener(), + IndexSearcher.getDefaultQueryCache(), + IndexSearcher.getDefaultQueryCachingPolicy(), + translogConfig, + TimeValue.timeValueMinutes(5), + config.getExternalRefreshListener(), + config.getInternalRefreshListener(), + null, + new NoneCircuitBreakerService(), + () -> UNASSIGNED_SEQ_NO, + () -> RetentionLeases.EMPTY, + primaryTerm::get, tombstoneDocSupplier()); expectThrows(EngineCreationFailureException.class, () -> new InternalEngine(brokenConfig)); @@ -5255,7 +5272,8 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { final IndexMetaData indexMetaData = IndexMetaData.builder(defaultSettings.getIndexMetaData()).settings(settings).build(); final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(indexMetaData); final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); - final AtomicReference> leasesHolder = new AtomicReference<>(Collections.emptyList()); + final AtomicLong retentionLeasesVersion = new AtomicLong(0); + final AtomicReference leasesHolder = new AtomicReference<>(RetentionLeases.EMPTY); final List operations = generateSingleDocHistory(true, randomFrom(VersionType.INTERNAL, VersionType.EXTERNAL), 2, 10, 300, "2"); Randomness.shuffle(operations); @@ -5277,6 +5295,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { globalCheckpoint.set(randomLongBetween(globalCheckpoint.get(), engine.getLocalCheckpointTracker().getCheckpoint())); } if (randomBoolean()) { + retentionLeasesVersion.incrementAndGet(); final int length = randomIntBetween(0, 8); final List leases = new ArrayList<>(length); for (int i = 0; i < length; i++) { @@ -5286,7 +5305,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { final String source = randomAlphaOfLength(8); leases.add(new RetentionLease(id, retainingSequenceNumber, timestamp, source)); } - leasesHolder.set(leases); + leasesHolder.set(new RetentionLeases(retentionLeasesVersion.get(), leases)); } if (rarely()) { settings.put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING.getKey(), randomLongBetween(0, 10)); @@ -5300,8 +5319,8 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { engine.flush(true, true); assertThat(Long.parseLong(engine.getLastCommittedSegmentInfos().userData.get(Engine.MIN_RETAINED_SEQNO)), equalTo(engine.getMinRetainedSeqNo())); - final Collection leases = leasesHolder.get(); - if (leases.isEmpty()) { + final RetentionLeases leases = leasesHolder.get(); + if (leases.retentionLeases().isEmpty()) { assertThat(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.RETENTION_LEASES), equalTo("")); } else { assertThat( diff --git a/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java b/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java index 310e83e9d2cef..71240571be8d4 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java @@ -24,15 +24,13 @@ import org.apache.lucene.search.Query; import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; @@ -51,13 +49,13 @@ public void testSoftDeletesRetentionLock() { for (int i = 0; i < retainingSequenceNumbers.length; i++) { retainingSequenceNumbers[i] = new AtomicLong(SequenceNumbers.UNASSIGNED_SEQ_NO); } - final Supplier> retentionLeasesSupplier = + final Supplier retentionLeasesSupplier = () -> { - final Set leases = new HashSet<>(retainingSequenceNumbers.length); + final List leases = new ArrayList<>(retainingSequenceNumbers.length); for (int i = 0; i < retainingSequenceNumbers.length; i++) { leases.add(new RetentionLease(Integer.toString(i), retainingSequenceNumbers[i].get(), 0L, "test")); } - return leases; + return new RetentionLeases(1, leases); }; long safeCommitCheckpoint = globalCheckpoint.get(); SoftDeletesPolicy policy = new SoftDeletesPolicy(globalCheckpoint::get, between(1, 10000), retainedOps, retentionLeasesSupplier); diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index 7a867027412e1..9467b40e92f6f 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.IndexSettingsModule; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -40,6 +41,8 @@ import java.util.stream.Collectors; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; @@ -69,13 +72,13 @@ public void testAddOrRenewRetentionLease() { minimumRetainingSequenceNumbers[i] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE); replicationTracker.addRetentionLease( Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i, ActionListener.wrap(() -> {})); - assertRetentionLeases(replicationTracker, i + 1, minimumRetainingSequenceNumbers, () -> 0L, true); + assertRetentionLeases(replicationTracker, i + 1, minimumRetainingSequenceNumbers, () -> 0L, 1 + i, true); } for (int i = 0; i < length; i++) { minimumRetainingSequenceNumbers[i] = randomLongBetween(minimumRetainingSequenceNumbers[i], Long.MAX_VALUE); replicationTracker.renewRetentionLease(Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i); - assertRetentionLeases(replicationTracker, length, minimumRetainingSequenceNumbers, () -> 0L, true); + assertRetentionLeases(replicationTracker, length, minimumRetainingSequenceNumbers, () -> 0L, 1 + length + i, true); } } @@ -96,7 +99,9 @@ public void testAddRetentionLeaseCausesRetentionLeaseSync() { assertFalse(Thread.holdsLock(reference.get())); invoked.set(true); assertThat( - leases.stream().collect(Collectors.toMap(RetentionLease::id, RetentionLease::retainingSequenceNumber)), + leases.retentionLeases() + .stream() + .collect(Collectors.toMap(RetentionLease::id, RetentionLease::retainingSequenceNumber)), equalTo(retentionLeases)); }); reference.set(replicationTracker); @@ -160,16 +165,19 @@ private void runExpirationTest(final boolean primaryMode) { if (primaryMode) { replicationTracker.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {})); } else { - replicationTracker.updateRetentionLeasesOnReplica( + final RetentionLeases retentionLeases = new RetentionLeases( + 1, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); + replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); } { - final Collection retentionLeases = replicationTracker.getRetentionLeases(); - assertThat(retentionLeases, hasSize(1)); - final RetentionLease retentionLease = retentionLeases.iterator().next(); + final RetentionLeases retentionLeases = replicationTracker.getRetentionLeases(); + assertThat(retentionLeases.version(), equalTo(1L)); + assertThat(retentionLeases.retentionLeases(), hasSize(1)); + final RetentionLease retentionLease = retentionLeases.retentionLeases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryMode); + assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, 1, primaryMode); } // renew the lease @@ -178,25 +186,28 @@ private void runExpirationTest(final boolean primaryMode) { if (primaryMode) { replicationTracker.renewRetentionLease("0", retainingSequenceNumbers[0], "test-0"); } else { - replicationTracker.updateRetentionLeasesOnReplica( + final RetentionLeases retentionLeases = new RetentionLeases( + 2, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); + replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); } { - final Collection retentionLeases = replicationTracker.getRetentionLeases(); - assertThat(retentionLeases, hasSize(1)); - final RetentionLease retentionLease = retentionLeases.iterator().next(); + final RetentionLeases retentionLeases = replicationTracker.getRetentionLeases(); + assertThat(retentionLeases.version(), equalTo(2L)); + assertThat(retentionLeases.retentionLeases(), hasSize(1)); + final RetentionLease retentionLease = retentionLeases.retentionLeases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryMode); + assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, primaryMode); } // now force the lease to expire currentTimeMillis.set(currentTimeMillis.get() + randomLongBetween(retentionLeaseMillis, Long.MAX_VALUE - currentTimeMillis.get())); if (primaryMode) { - assertRetentionLeases(replicationTracker, 0, retainingSequenceNumbers, currentTimeMillis::get, true); + assertRetentionLeases(replicationTracker, 0, retainingSequenceNumbers, currentTimeMillis::get, 3, true); } else { // leases do not expire on replicas until synced from the primary - assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, false); + assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, false); } } @@ -223,7 +234,9 @@ public void testRetentionLeaseExpirationCausesRetentionLeaseSync() { assertFalse(Thread.holdsLock(reference.get())); invoked.set(true); assertThat( - leases.stream().collect(Collectors.toMap(RetentionLease::id, ReplicationTrackerRetentionLeaseTests::toTuple)), + leases.retentionLeases() + .stream() + .collect(Collectors.toMap(RetentionLease::id, ReplicationTrackerRetentionLeaseTests::toTuple)), equalTo(retentionLeases)); }); reference.set(replicationTracker); @@ -235,11 +248,14 @@ public void testRetentionLeaseExpirationCausesRetentionLeaseSync() { replicationTracker.activatePrimaryMode(SequenceNumbers.NO_OPS_PERFORMED); final int length = randomIntBetween(0, 8); + long version = 0; for (int i = 0; i < length; i++) { final String id = randomAlphaOfLength(8); final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE); retentionLeases.put(id, Tuple.tuple(retainingSequenceNumber, currentTimeMillis.get())); replicationTracker.addRetentionLease(id, retainingSequenceNumber, "test", ActionListener.wrap(() -> {})); + version++; + assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); // assert that the new retention lease callback was invoked assertTrue(invoked.get()); @@ -248,6 +264,8 @@ public void testRetentionLeaseExpirationCausesRetentionLeaseSync() { currentTimeMillis.set(1 + currentTimeMillis.get()); retentionLeases.put(id, Tuple.tuple(retainingSequenceNumber, currentTimeMillis.get())); replicationTracker.renewRetentionLease(id, retainingSequenceNumber, "test"); + version++; + assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); // reset the invocation marker so that we can assert the callback was invoked if any leases are expired assertFalse(invoked.get()); @@ -260,16 +278,67 @@ public void testRetentionLeaseExpirationCausesRetentionLeaseSync() { .map(Map.Entry::getKey) .collect(Collectors.toList()); expiredIds.forEach(retentionLeases::remove); + if (expiredIds.isEmpty() == false) { + version++; + } currentTimeMillis.set(currentTimeMillis.get() + currentTimeMillisIncrement); // getting the leases has the side effect of calculating which leases are expired and invoking the sync callback - final Collection current = replicationTracker.getRetentionLeases(); + final RetentionLeases current = replicationTracker.getRetentionLeases(); + assertThat(current.version(), equalTo(version)); // the current leases should equal our tracking map assertThat( - current.stream().collect(Collectors.toMap(RetentionLease::id, ReplicationTrackerRetentionLeaseTests::toTuple)), + current.retentionLeases() + .stream() + .collect(Collectors.toMap(RetentionLease::id, ReplicationTrackerRetentionLeaseTests::toTuple)), equalTo(retentionLeases)); // the callback should only be invoked if there were expired leases assertThat(invoked.get(), equalTo(expiredIds.isEmpty() == false)); } + assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); + } + + public void testReplicaIgnoresOlderRetentionLeasesVersion() { + final AllocationId allocationId = AllocationId.newInitializing(); + final ReplicationTracker replicationTracker = new ReplicationTracker( + new ShardId("test", "_na", 0), + allocationId.getId(), + IndexSettingsModule.newIndexSettings("test", Settings.EMPTY), + UNASSIGNED_SEQ_NO, + value -> {}, + () -> 0L, + (leases, listener) -> {}); + replicationTracker.updateFromMaster( + randomNonNegativeLong(), + Collections.singleton(allocationId.getId()), + routingTable(Collections.emptySet(), allocationId), + Collections.emptySet()); + final int length = randomIntBetween(0, 8); + final List retentionLeasesCollection = new ArrayList<>(length); + long version = 0; + for (int i = 0; i < length; i++) { + final int innerLength = randomIntBetween(0, 8); + final Collection retentionLeases = new ArrayList<>(); + for (int j = 0; j < innerLength; j++) { + retentionLeases.add( + new RetentionLease(i + "-" + j, randomNonNegativeLong(), randomNonNegativeLong(), randomAlphaOfLength(8))); + version++; + } + retentionLeasesCollection.add(new RetentionLeases(version, retentionLeases)); + } + + Collections.shuffle(retentionLeasesCollection, random()); + for (final RetentionLeases retentionLeases : retentionLeasesCollection) { + replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); + } + assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); + if (length == 0) { + assertThat(replicationTracker.getRetentionLeases().retentionLeases(), empty()); + } else { + final RetentionLeases expected = retentionLeasesCollection.get(length - 1); + assertThat( + replicationTracker.getRetentionLeases().retentionLeases(), + contains(expected.retentionLeases().toArray(new RetentionLease[0]))); + } } private static Tuple toTuple(final RetentionLease retentionLease) { @@ -281,10 +350,12 @@ private void assertRetentionLeases( final int size, final long[] minimumRetainingSequenceNumbers, final LongSupplier currentTimeMillisSupplier, + final long version, final boolean primaryMode) { - final Collection retentionLeases = replicationTracker.getRetentionLeases(); + final RetentionLeases retentionLeases = replicationTracker.getRetentionLeases(); + assertThat(retentionLeases.version(), equalTo(version)); final Map idToRetentionLease = new HashMap<>(); - for (final RetentionLease retentionLease : retentionLeases) { + for (final RetentionLease retentionLease : retentionLeases.retentionLeases()) { idToRetentionLease.put(retentionLease.id(), retentionLease); } diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerTests.java index b61e3f647b9d2..ff6974244f2e8 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerTests.java @@ -686,7 +686,7 @@ public void testPrimaryContextHandoff() throws IOException { final AllocationId primaryAllocationId = clusterState.routingTable.primaryShard().allocationId(); final LongConsumer onUpdate = updatedGlobalCheckpoint -> {}; final long globalCheckpoint = UNASSIGNED_SEQ_NO; - final BiConsumer, ActionListener> onNewRetentionLease = + final BiConsumer> onNewRetentionLease = (leases, listener) -> {}; ReplicationTracker oldPrimary = new ReplicationTracker( shardId, primaryAllocationId.getId(), indexSettings, globalCheckpoint, onUpdate, () -> 0L, onNewRetentionLease); diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java index 0cd85ef60f21a..ab92d5ad2326d 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java @@ -43,7 +43,6 @@ import org.elasticsearch.transport.TransportService; import org.mockito.ArgumentCaptor; -import java.util.Collection; import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; @@ -114,10 +113,8 @@ public void testRetentionLeaseSyncActionOnPrimary() { shardStateAction, new ActionFilters(Collections.emptySet()), new IndexNameExpressionResolver()); - @SuppressWarnings("unchecked") final Collection retentionLeases = - (Collection) mock(Collection.class); - final RetentionLeaseSyncAction.Request request = - new RetentionLeaseSyncAction.Request(indexShard.shardId(), retentionLeases); + final RetentionLeases retentionLeases = mock(RetentionLeases.class); + final RetentionLeaseSyncAction.Request request = new RetentionLeaseSyncAction.Request(indexShard.shardId(), retentionLeases); final TransportWriteAction.WritePrimaryResult result = action.shardOperationOnPrimary(request, indexShard); @@ -155,10 +152,8 @@ public void testRetentionLeaseSyncActionOnReplica() { shardStateAction, new ActionFilters(Collections.emptySet()), new IndexNameExpressionResolver()); - @SuppressWarnings("unchecked") final Collection retentionLeases = - (Collection) mock(Collection.class); - final RetentionLeaseSyncAction.Request request = - new RetentionLeaseSyncAction.Request(indexShard.shardId(), retentionLeases); + final RetentionLeases retentionLeases = mock(RetentionLeases.class); + final RetentionLeaseSyncAction.Request request = new RetentionLeaseSyncAction.Request(indexShard.shardId(), retentionLeases); final TransportWriteAction.WriteReplicaResult result = action.shardOperationOnReplica(request, indexShard); // the retention leases on the shard should be updated @@ -190,8 +185,7 @@ public void testRetentionLeaseSyncExecution() { final Logger retentionLeaseSyncActionLogger = mock(Logger.class); - @SuppressWarnings("unchecked") final Collection retentionLeases = - (Collection) mock(Collection.class); + final RetentionLeases retentionLeases = mock(RetentionLeases.class); final AtomicBoolean invoked = new AtomicBoolean(); final RetentionLeaseSyncAction action = new RetentionLeaseSyncAction( Settings.EMPTY, diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java index 7d6e5fa2dc5a6..3327d375b375a 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java @@ -32,7 +32,6 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.threadpool.ThreadPool; -import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.concurrent.CountDownLatch; @@ -75,7 +74,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { latch.await(); // check retention leases have been committed on the primary - final Collection primaryCommittedRetentionLeases = RetentionLease.decodeRetentionLeases( + final RetentionLeases primaryCommittedRetentionLeases = RetentionLease.decodeRetentionLeases( primary.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES)); assertThat(currentRetentionLeases, equalTo(toMap(primaryCommittedRetentionLeases))); @@ -90,7 +89,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); // check retention leases have been committed on the replica - final Collection replicaCommittedRetentionLeases = RetentionLease.decodeRetentionLeases( + final RetentionLeases replicaCommittedRetentionLeases = RetentionLease.decodeRetentionLeases( replica.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES)); assertThat(currentRetentionLeases, equalTo(toMap(replicaCommittedRetentionLeases))); } @@ -134,14 +133,14 @@ public void testRetentionLeasesSyncOnExpiration() throws Exception { final IndexShard replica = internalCluster() .getInstance(IndicesService.class, replicaShardNodeName) .getShardOrNull(new ShardId(resolveIndex("index"), 0)); - assertThat(replica.getRetentionLeases(), hasItem(currentRetentionLease)); + assertThat(replica.getRetentionLeases().retentionLeases(), hasItem(currentRetentionLease)); } // sleep long enough that *possibly* the current retention lease has expired, and certainly that any previous have final long later = System.nanoTime(); Thread.sleep(Math.max(0, retentionLeaseTimeToLive.millis() - TimeUnit.NANOSECONDS.toMillis(later - now))); - final Collection currentRetentionLeases = primary.getRetentionLeases(); - assertThat(currentRetentionLeases, anyOf(empty(), contains(currentRetentionLease))); + final RetentionLeases currentRetentionLeases = primary.getRetentionLeases(); + assertThat(currentRetentionLeases.retentionLeases(), anyOf(empty(), contains(currentRetentionLease))); /* * Check that expiration of retention leases has been synced to all replicas. We have to assert busy since syncing happens in @@ -154,18 +153,23 @@ public void testRetentionLeasesSyncOnExpiration() throws Exception { final IndexShard replica = internalCluster() .getInstance(IndicesService.class, replicaShardNodeName) .getShardOrNull(new ShardId(resolveIndex("index"), 0)); - if (currentRetentionLeases.isEmpty()) { - assertThat(replica.getRetentionLeases(), empty()); + if (currentRetentionLeases.retentionLeases().isEmpty()) { + assertThat(replica.getRetentionLeases().retentionLeases(), empty()); } else { - assertThat(replica.getRetentionLeases(), contains(currentRetentionLeases.toArray(new RetentionLease[0]))); + assertThat( + replica.getRetentionLeases().retentionLeases(), + contains(currentRetentionLeases.retentionLeases().toArray(new RetentionLease[0]))); } } }); } } - private static Map toMap(final Collection replicaCommittedRetentionLeases) { - return replicaCommittedRetentionLeases.stream().collect(Collectors.toMap(RetentionLease::id, Function.identity())); + private static Map toMap(final RetentionLeases replicaCommittedRetentionLeases) { + return replicaCommittedRetentionLeases + .retentionLeases() + .stream() + .collect(Collectors.toMap(RetentionLease::id, Function.identity())); } } diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java index 500393f2cfac2..7fc2f3e8fd95e 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java @@ -25,7 +25,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; @@ -109,6 +108,7 @@ public void testRetentionLeaseEncoding() { } public void testRetentionLeasesEncoding() { + final long version = randomNonNegativeLong(); final int length = randomIntBetween(0, 8); final List retentionLeases = new ArrayList<>(length); for (int i = 0; i < length; i++) { @@ -119,12 +119,13 @@ public void testRetentionLeasesEncoding() { final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, timestamp, source); retentionLeases.add(retentionLease); } - final Collection decodedRetentionLeases = - RetentionLease.decodeRetentionLeases(RetentionLease.encodeRetentionLeases(retentionLeases)); + final RetentionLeases decodedRetentionLeases = + RetentionLease.decodeRetentionLeases(RetentionLease.encodeRetentionLeases(new RetentionLeases(version, retentionLeases))); + assertThat(decodedRetentionLeases.version(), equalTo(version)); if (length == 0) { - assertThat(decodedRetentionLeases, empty()); + assertThat(decodedRetentionLeases.retentionLeases(), empty()); } else { - assertThat(decodedRetentionLeases, contains(retentionLeases.toArray(new RetentionLease[0]))); + assertThat(decodedRetentionLeases.retentionLeases(), contains(retentionLeases.toArray(new RetentionLease[0]))); } } diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java index cd7d2a2c12cb8..e506811390a3a 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java @@ -30,11 +30,11 @@ import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.InternalEngineFactory; import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -82,13 +82,13 @@ public void testAddOrRenewRetentionLease() throws IOException { minimumRetainingSequenceNumbers[i] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE); indexShard.addRetentionLease( Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i, ActionListener.wrap(() -> {})); - assertRetentionLeases(indexShard, i + 1, minimumRetainingSequenceNumbers, () -> 0L, true); + assertRetentionLeases(indexShard, i + 1, minimumRetainingSequenceNumbers, () -> 0L, 1 + i, true); } for (int i = 0; i < length; i++) { minimumRetainingSequenceNumbers[i] = randomLongBetween(minimumRetainingSequenceNumbers[i], Long.MAX_VALUE); indexShard.renewRetentionLease(Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i); - assertRetentionLeases(indexShard, length, minimumRetainingSequenceNumbers, () -> 0L, true); + assertRetentionLeases(indexShard, length, minimumRetainingSequenceNumbers, () -> 0L, 1 + length + i, true); } } finally { closeShards(indexShard); @@ -117,16 +117,19 @@ private void runExpirationTest(final boolean primary) throws IOException { if (primary) { indexShard.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {})); } else { - indexShard.updateRetentionLeasesOnReplica( + final RetentionLeases retentionLeases = new RetentionLeases( + 1, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); + indexShard.updateRetentionLeasesOnReplica(retentionLeases); } { - final Collection retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); - assertThat(retentionLeases, hasSize(1)); - final RetentionLease retentionLease = retentionLeases.iterator().next(); + final RetentionLeases retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); + assertThat(retentionLeases.version(), equalTo(1L)); + assertThat(retentionLeases.retentionLeases(), hasSize(1)); + final RetentionLease retentionLease = retentionLeases.retentionLeases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, primary); + assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, 1, primary); } // renew the lease @@ -135,25 +138,28 @@ private void runExpirationTest(final boolean primary) throws IOException { if (primary) { indexShard.renewRetentionLease("0", retainingSequenceNumbers[0], "test-0"); } else { - indexShard.updateRetentionLeasesOnReplica( + final RetentionLeases retentionLeases = new RetentionLeases( + 2, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); + indexShard.updateRetentionLeasesOnReplica(retentionLeases); } { - final Collection retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); - assertThat(retentionLeases, hasSize(1)); - final RetentionLease retentionLease = retentionLeases.iterator().next(); + final RetentionLeases retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); + assertThat(retentionLeases.version(), equalTo(2L)); + assertThat(retentionLeases.retentionLeases(), hasSize(1)); + final RetentionLease retentionLease = retentionLeases.retentionLeases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, primary); + assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, primary); } // now force the lease to expire currentTimeMillis.set( currentTimeMillis.get() + randomLongBetween(retentionLeaseMillis, Long.MAX_VALUE - currentTimeMillis.get())); if (primary) { - assertRetentionLeases(indexShard, 0, retainingSequenceNumbers, currentTimeMillis::get, true); + assertRetentionLeases(indexShard, 0, retainingSequenceNumbers, currentTimeMillis::get, 3, true); } else { - assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, false); + assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, false); } } finally { closeShards(indexShard); @@ -187,11 +193,14 @@ public void testCommit() throws IOException { // the committed retention leases should equal our current retention leases final SegmentInfos segmentCommitInfos = indexShard.store().readLastCommittedSegmentsInfo(); assertTrue(segmentCommitInfos.getUserData().containsKey(Engine.RETENTION_LEASES)); - final Collection retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); - if (retentionLeases.isEmpty()) { - assertThat(IndexShard.getRetentionLeases(segmentCommitInfos), empty()); + final RetentionLeases retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); + final RetentionLeases committedRetentionLeases = IndexShard.getRetentionLeases(segmentCommitInfos); + if (retentionLeases.retentionLeases().isEmpty()) { + assertThat(committedRetentionLeases.version(), equalTo(0L)); + assertThat(committedRetentionLeases.retentionLeases(), empty()); } else { - assertThat(IndexShard.getRetentionLeases(segmentCommitInfos), contains(retentionLeases.toArray(new RetentionLease[0]))); + assertThat(committedRetentionLeases.version(), equalTo((long) length)); + assertThat(retentionLeases.retentionLeases(), contains(retentionLeases.retentionLeases().toArray(new RetentionLease[0]))); } // when we recover, we should recover the retention leases @@ -200,12 +209,15 @@ public void testCommit() throws IOException { ShardRoutingHelper.initWithSameId(indexShard.routingEntry(), RecoverySource.ExistingStoreRecoverySource.INSTANCE)); try { recoverShardFromStore(recoveredShard); - if (retentionLeases.isEmpty()) { - assertThat(recoveredShard.getEngine().config().retentionLeasesSupplier().get(), empty()); + final RetentionLeases recoveredRetentionLeases = recoveredShard.getEngine().config().retentionLeasesSupplier().get(); + if (retentionLeases.retentionLeases().isEmpty()) { + assertThat(recoveredRetentionLeases.version(), equalTo(0L)); + assertThat(recoveredRetentionLeases.retentionLeases(), empty()); } else { + assertThat(recoveredRetentionLeases.version(), equalTo((long) length)); assertThat( - recoveredShard.getEngine().config().retentionLeasesSupplier().get(), - contains(retentionLeases.toArray(new RetentionLease[0]))); + recoveredRetentionLeases.retentionLeases(), + contains(retentionLeases.retentionLeases().toArray(new RetentionLease[0]))); } } finally { closeShards(recoveredShard); @@ -220,10 +232,12 @@ private void assertRetentionLeases( final int size, final long[] minimumRetainingSequenceNumbers, final LongSupplier currentTimeMillisSupplier, + final long version, final boolean primary) { - final Collection retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); + final RetentionLeases retentionLeases = indexShard.getEngine().config().retentionLeasesSupplier().get(); + assertThat(retentionLeases.version(), equalTo(version)); final Map idToRetentionLease = new HashMap<>(); - for (final RetentionLease retentionLease : retentionLeases) { + for (final RetentionLease retentionLease : retentionLeases.retentionLeases()) { idToRetentionLease.put(retentionLease.id(), retentionLease); } diff --git a/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java b/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java index 53c3e86ee01fb..c80b3b5074921 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java @@ -50,6 +50,7 @@ import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SeqNoFieldMapper; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.store.Store; import org.elasticsearch.index.translog.Translog; @@ -122,12 +123,30 @@ public void onFailedEngine(String reason, @Nullable Exception e) { final String translogUUID = Translog.createEmptyTranslog(translogConfig.getTranslogPath(), SequenceNumbers.NO_OPS_PERFORMED, shardId, primaryTerm); store.associateIndexWithNewTranslog(translogUUID); - EngineConfig config = new EngineConfig(shardId, allocationId, threadPool, - indexSettings, null, store, newMergePolicy(), iwc.getAnalyzer(), iwc.getSimilarity(), new CodecService(null, logger), - eventListener, IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), translogConfig, - TimeValue.timeValueMinutes(5), Collections.singletonList(listeners), Collections.emptyList(), null, - new NoneCircuitBreakerService(), () -> SequenceNumbers.NO_OPS_PERFORMED, Collections::emptyList, - () -> primaryTerm, EngineTestCase.tombstoneDocSupplier()); + EngineConfig config = new EngineConfig( + shardId, + allocationId, + threadPool, + indexSettings, + null, + store, + newMergePolicy(), + iwc.getAnalyzer(), + iwc.getSimilarity(), + new CodecService(null, logger), + eventListener, + IndexSearcher.getDefaultQueryCache(), + IndexSearcher.getDefaultQueryCachingPolicy(), + translogConfig, + TimeValue.timeValueMinutes(5), + Collections.singletonList(listeners), + Collections.emptyList(), + null, + new NoneCircuitBreakerService(), + () -> SequenceNumbers.NO_OPS_PERFORMED, + () -> RetentionLeases.EMPTY, + () -> primaryTerm, + EngineTestCase.tombstoneDocSupplier()); engine = new InternalEngine(config); engine.initializeMaxSeqNoOfUpdatesOrDeletes(); engine.recoverFromTranslog((e, s) -> 0, Long.MAX_VALUE); diff --git a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java index 35667b0f87a1c..e4b0f1e00fe81 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java @@ -84,7 +84,7 @@ import org.elasticsearch.index.mapper.VersionFieldMapper; import org.elasticsearch.index.seqno.LocalCheckpointTracker; import org.elasticsearch.index.seqno.ReplicationTracker; -import org.elasticsearch.index.seqno.RetentionLease; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.store.Store; @@ -105,7 +105,6 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -586,7 +585,7 @@ public EngineConfig config(IndexSettings indexSettings, Store store, Path transl refreshListener, indexSort, globalCheckpointSupplier, - globalCheckpointSupplier == null ? null : Collections::emptyList); + globalCheckpointSupplier == null ? null : () -> RetentionLeases.EMPTY); } public EngineConfig config( @@ -597,7 +596,7 @@ public EngineConfig config( final ReferenceManager.RefreshListener refreshListener, final Sort indexSort, final LongSupplier globalCheckpointSupplier, - final Supplier> retentionLeasesSupplier) { + final Supplier retentionLeasesSupplier) { return config( indexSettings, store, @@ -625,7 +624,7 @@ public EngineConfig config(IndexSettings indexSettings, Store store, Path transl internalRefreshListener, indexSort, maybeGlobalCheckpointSupplier, - maybeGlobalCheckpointSupplier == null ? null : Collections::emptyList, + maybeGlobalCheckpointSupplier == null ? null : () -> RetentionLeases.EMPTY, breakerService); } @@ -638,7 +637,7 @@ public EngineConfig config( final ReferenceManager.RefreshListener internalRefreshListener, final Sort indexSort, final @Nullable LongSupplier maybeGlobalCheckpointSupplier, - final @Nullable Supplier> maybeRetentionLeasesSupplier, + final @Nullable Supplier maybeRetentionLeasesSupplier, final CircuitBreakerService breakerService) { final IndexWriterConfig iwc = newIndexWriterConfig(); final TranslogConfig translogConfig = new TranslogConfig(shardId, translogPath, indexSettings, BigArrays.NON_RECYCLING_INSTANCE); @@ -648,7 +647,7 @@ public EngineConfig config( final List intRefreshListenerList = internalRefreshListener == null ? emptyList() : Collections.singletonList(internalRefreshListener); final LongSupplier globalCheckpointSupplier; - final Supplier> retentionLeasesSupplier; + final Supplier retentionLeasesSupplier; if (maybeGlobalCheckpointSupplier == null) { assert maybeRetentionLeasesSupplier == null; final ReplicationTracker replicationTracker = new ReplicationTracker( diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/index/engine/FollowingEngineTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/index/engine/FollowingEngineTests.java index bccc5fed8364e..df406a4c09a68 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/index/engine/FollowingEngineTests.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/index/engine/FollowingEngineTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.index.engine.TranslogHandler; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.ParsedDocument; +import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.store.Store; @@ -270,7 +271,7 @@ public void onFailedEngine(String reason, Exception e) { null, new NoneCircuitBreakerService(), globalCheckpoint::longValue, - Collections::emptyList, + () -> RetentionLeases.EMPTY, () -> primaryTerm.get(), EngineTestCase.tombstoneDocSupplier()); } From 38dffb540a8d4d8f1e67cbc6c190b40dd8362d56 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 29 Jan 2019 10:11:35 -0500 Subject: [PATCH 02/26] Add Javadocs --- .../index/seqno/RetentionLease.java | 3 +- .../index/seqno/RetentionLeases.java | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java index b8626793f08ca..51a25950b383a 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java @@ -168,7 +168,8 @@ static String encodeRetentionLease(final RetentionLease retentionLease) { /** * Encodes a collection of retention leases as a string. This encoding can be decoed by {@link #decodeRetentionLeases(String)}. The - * encoding is a comma-separated encoding of each retention lease as encoded by {@link #encodeRetentionLease(RetentionLease)}. + * encoding is a comma-separated encoding of each retention lease as encoded by {@link #encodeRetentionLease(RetentionLease)}, prefixed + * by the version of the retention lease collection. * * @param retentionLeases the retention leases * @return the encoding of the retention leases diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 77876e11f5755..41379d76d76b8 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -10,22 +10,47 @@ import java.util.Collections; import java.util.Objects; +/** + * Represents a versioned collection of retention leases. We version the collection of retention leases to ensure that sync requests that + * arrive out of order on the replica, using the version to ensure that older sync requests are rejected. + */ public class RetentionLeases implements Writeable { private final long version; + /** + * The version of this retention lease collection. The version is managed on the primary and incremented any time that a retention lease + * is added, renewed, or when retention leases expire. + * + * @return the version of this retention lease collection + */ public long version() { return version; } private final Collection retentionLeases; + /** + * The underlying collection of retention leases + * + * @return the retention leases + */ public Collection retentionLeases() { return retentionLeases; } + /** + * Represents an empty an un-versioned retention lease collection. This is used when no retention lease collection is found in the + * commit point + */ public static RetentionLeases EMPTY = new RetentionLeases(0, Collections.emptyList()); + /** + * Constructs a new retention lease collection with the specified version and underlying collection of retention leases. + * + * @param version the version of this retention lease collection + * @param retentionLeases the retention leases + */ public RetentionLeases(final long version, final Collection retentionLeases) { if (version < 0) { throw new IllegalArgumentException("version must be non-negative but was [" + version + "]"); @@ -35,15 +60,37 @@ public RetentionLeases(final long version, final Collection rete this.retentionLeases = Collections.unmodifiableCollection(new ArrayList<>(retentionLeases)); } + /** + * Constructs a new retention lease collection from a stream. The retention lease collection should have been written via + * {@link #writeTo(StreamOutput)}. + * + * @param in the stream to construct the retention lease collection from + * @throws IOException if an I/O exception occurs reading from the stream + */ public RetentionLeases(final StreamInput in) throws IOException { version = in.readVLong(); retentionLeases = in.readList(RetentionLease::new); } + /** + * Writes a retention lease collection to a stream in a manner suitable for later reconstruction via + * {@link #RetentionLeases(StreamInput)} (StreamInput)}. + * + * @param out the stream to write the retention lease collection to + * @throws IOException if an I/O exception occurs writing to the stream + */ @Override public void writeTo(final StreamOutput out) throws IOException { out.writeVLong(version); out.writeCollection(retentionLeases); } + @Override + public String toString() { + return "RetentionLeases{" + + "version=" + version + + ", retentionLeases=" + retentionLeases + + '}'; + } + } From c715b5e4bd68b15a26aaadf33e34dba6a54246e3 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 29 Jan 2019 10:27:27 -0500 Subject: [PATCH 03/26] License header --- .../index/seqno/RetentionLeases.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 41379d76d76b8..40b71ea3075e1 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -1,3 +1,22 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.elasticsearch.index.seqno; import org.elasticsearch.common.io.stream.StreamInput; From f8e8547a6bce080a39e1d8a771be06d3248ebf35 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 29 Jan 2019 10:32:24 -0500 Subject: [PATCH 04/26] Relocate methods --- .../index/engine/InternalEngine.java | 3 +- .../index/seqno/RetentionLease.java | 48 ---------------- .../index/seqno/RetentionLeases.java | 49 ++++++++++++++++ .../elasticsearch/index/shard/IndexShard.java | 2 +- .../index/engine/InternalEngineTests.java | 2 +- .../index/seqno/RetentionLeaseSyncIT.java | 4 +- .../index/seqno/RetentionLeaseTests.java | 22 -------- .../index/seqno/RetentionLeasesTests.java | 56 +++++++++++++++++++ 8 files changed, 110 insertions(+), 76 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java 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 eda33c9698432..74a07150f94a1 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -75,7 +75,6 @@ import org.elasticsearch.index.merge.MergeStats; import org.elasticsearch.index.merge.OnGoingMerge; import org.elasticsearch.index.seqno.LocalCheckpointTracker; -import org.elasticsearch.index.seqno.RetentionLease; import org.elasticsearch.index.seqno.RetentionLeases; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbers; @@ -2345,7 +2344,7 @@ protected void commitIndexWriter(final IndexWriter writer, final Translog transl */ final Tuple retentionPolicy = softDeletesPolicy.getRetentionPolicy(); commitData.put(Engine.MIN_RETAINED_SEQNO, Long.toString(retentionPolicy.v1())); - commitData.put(Engine.RETENTION_LEASES, RetentionLease.encodeRetentionLeases(retentionPolicy.v2())); + commitData.put(Engine.RETENTION_LEASES, RetentionLeases.encodeRetentionLeases(retentionPolicy.v2())); } logger.trace("committing writer with commit data [{}]", commitData); return commitData.entrySet().iterator(); diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java index 51a25950b383a..9c9a8e541f63b 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java @@ -25,8 +25,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; import java.util.Locale; import java.util.Objects; import java.util.stream.Collectors; @@ -166,23 +164,6 @@ static String encodeRetentionLease(final RetentionLease retentionLease) { retentionLease.source()); } - /** - * Encodes a collection of retention leases as a string. This encoding can be decoed by {@link #decodeRetentionLeases(String)}. The - * encoding is a comma-separated encoding of each retention lease as encoded by {@link #encodeRetentionLease(RetentionLease)}, prefixed - * by the version of the retention lease collection. - * - * @param retentionLeases the retention leases - * @return the encoding of the retention leases - */ - public static String encodeRetentionLeases(final RetentionLeases retentionLeases) { - Objects.requireNonNull(retentionLeases); - return String.format( - Locale.ROOT, - "version:%d;%s", - retentionLeases.version(), - retentionLeases.retentionLeases().stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(","))); - } - /** * Decodes a retention lease encoded by {@link #encodeRetentionLease(RetentionLease)}. * @@ -204,35 +185,6 @@ static RetentionLease decodeRetentionLease(final String encodedRetentionLease) { return new RetentionLease(id, retainingSequenceNumber, timestamp, source); } - /** - * Decodes retention leases encoded by {@link #encodeRetentionLeases(RetentionLeases)}. - * - * @param encodedRetentionLeases an encoded collection of retention leases - * @return the decoded retention leases - */ - public static RetentionLeases decodeRetentionLeases(final String encodedRetentionLeases) { - Objects.requireNonNull(encodedRetentionLeases); - if (encodedRetentionLeases.isEmpty()) { - return RetentionLeases.EMPTY; - } - assert encodedRetentionLeases.matches("version:\\d+;.*") : encodedRetentionLeases; - final int firstSemicolon = encodedRetentionLeases.indexOf(";"); - final long version = Long.parseLong(encodedRetentionLeases.substring("version:".length(), firstSemicolon)); - final Collection retentionLeases; - if (firstSemicolon + 1 == encodedRetentionLeases.length()) { - retentionLeases = Collections.emptyList(); - } else { - assert Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) - .allMatch(s -> s.matches("id:[^:;,]+;retaining_seq_no:\\d+;timestamp:\\d+;source:[^:;,]+")) - : encodedRetentionLeases; - retentionLeases = Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) - .map(RetentionLease::decodeRetentionLease) - .collect(Collectors.toList()); - } - - return new RetentionLeases(version, retentionLeases); - } - @Override public boolean equals(final Object o) { if (this == o) return true; diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 40b71ea3075e1..9da7cc6e9f12e 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -25,9 +25,12 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Locale; import java.util.Objects; +import java.util.stream.Collectors; /** * Represents a versioned collection of retention leases. We version the collection of retention leases to ensure that sync requests that @@ -104,6 +107,52 @@ public void writeTo(final StreamOutput out) throws IOException { out.writeCollection(retentionLeases); } + /** + * Encodes a retention lease collection as a string. This encoding can be decoded by + * {@link RetentionLeases#decodeRetentionLeases(String)}. The encoding is a comma-separated encoding of each retention lease as encoded + * by {@link RetentionLease#encodeRetentionLease(RetentionLease)}, prefixed by the version of the retention lease collection. + * + * @param retentionLeases the retention lease collection + * @return the encoding of the retention lease collection + */ + public static String encodeRetentionLeases(final RetentionLeases retentionLeases) { + Objects.requireNonNull(retentionLeases); + return String.format( + Locale.ROOT, + "version:%d;%s", + retentionLeases.version(), + retentionLeases.retentionLeases().stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(","))); + } + + /** + * Decodes retention leases encoded by {@link #encodeRetentionLeases(RetentionLeases)}. + * + * @param encodedRetentionLeases an encoded retention lease collection + * @return the decoded retention lease collection + */ + public static RetentionLeases decodeRetentionLeases(final String encodedRetentionLeases) { + Objects.requireNonNull(encodedRetentionLeases); + if (encodedRetentionLeases.isEmpty()) { + return EMPTY; + } + assert encodedRetentionLeases.matches("version:\\d+;.*") : encodedRetentionLeases; + final int firstSemicolon = encodedRetentionLeases.indexOf(";"); + final long version = Long.parseLong(encodedRetentionLeases.substring("version:".length(), firstSemicolon)); + final Collection retentionLeases; + if (firstSemicolon + 1 == encodedRetentionLeases.length()) { + retentionLeases = Collections.emptyList(); + } else { + assert Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) + .allMatch(s -> s.matches("id:[^:;,]+;retaining_seq_no:\\d+;timestamp:\\d+;source:[^:;,]+")) + : encodedRetentionLeases; + retentionLeases = Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) + .map(RetentionLease::decodeRetentionLease) + .collect(Collectors.toList()); + } + + return new RetentionLeases(version, retentionLeases); + } + @Override public String toString() { return "RetentionLeases{" + diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 5e88d09e2f040..4ee0c259331fa 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -1447,7 +1447,7 @@ static RetentionLeases getRetentionLeases(final SegmentInfos segmentInfos) { if (committedRetentionLeases == null) { return RetentionLeases.EMPTY; } - return RetentionLease.decodeRetentionLeases(committedRetentionLeases); + return RetentionLeases.decodeRetentionLeases(committedRetentionLeases); } private void trimUnsafeCommits() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index b45589ea5c47e..0d642e3dc62d0 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -5325,7 +5325,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { } else { assertThat( engine.getLastCommittedSegmentInfos().getUserData().get(Engine.RETENTION_LEASES), - equalTo(RetentionLease.encodeRetentionLeases(leases))); + equalTo(RetentionLeases.encodeRetentionLeases(leases))); } } if (rarely()) { diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java index 3327d375b375a..88497e5f673a9 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java @@ -74,7 +74,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { latch.await(); // check retention leases have been committed on the primary - final RetentionLeases primaryCommittedRetentionLeases = RetentionLease.decodeRetentionLeases( + final RetentionLeases primaryCommittedRetentionLeases = RetentionLeases.decodeRetentionLeases( primary.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES)); assertThat(currentRetentionLeases, equalTo(toMap(primaryCommittedRetentionLeases))); @@ -89,7 +89,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); // check retention leases have been committed on the replica - final RetentionLeases replicaCommittedRetentionLeases = RetentionLease.decodeRetentionLeases( + final RetentionLeases replicaCommittedRetentionLeases = RetentionLeases.decodeRetentionLeases( replica.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES)); assertThat(currentRetentionLeases, equalTo(toMap(replicaCommittedRetentionLeases))); } diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java index 7fc2f3e8fd95e..f5f74a165e730 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java @@ -107,26 +107,4 @@ public void testRetentionLeaseEncoding() { assertThat(RetentionLease.decodeRetentionLease(RetentionLease.encodeRetentionLease(retentionLease)), equalTo(retentionLease)); } - public void testRetentionLeasesEncoding() { - final long version = randomNonNegativeLong(); - final int length = randomIntBetween(0, 8); - final List retentionLeases = new ArrayList<>(length); - for (int i = 0; i < length; i++) { - final String id = randomAlphaOfLength(8); - final long retainingSequenceNumber = randomNonNegativeLong(); - final long timestamp = randomNonNegativeLong(); - final String source = randomAlphaOfLength(8); - final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, timestamp, source); - retentionLeases.add(retentionLease); - } - final RetentionLeases decodedRetentionLeases = - RetentionLease.decodeRetentionLeases(RetentionLease.encodeRetentionLeases(new RetentionLeases(version, retentionLeases))); - assertThat(decodedRetentionLeases.version(), equalTo(version)); - if (length == 0) { - assertThat(decodedRetentionLeases.retentionLeases(), empty()); - } else { - assertThat(decodedRetentionLeases.retentionLeases(), contains(retentionLeases.toArray(new RetentionLease[0]))); - } - } - } diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java new file mode 100644 index 0000000000000..84f3464ff4413 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java @@ -0,0 +1,56 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.seqno; + +import org.elasticsearch.test.ESTestCase; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.*; + +public class RetentionLeasesTests extends ESTestCase { + + public void testRetentionLeasesEncoding() { + final long version = randomNonNegativeLong(); + final int length = randomIntBetween(0, 8); + final List retentionLeases = new ArrayList<>(length); + for (int i = 0; i < length; i++) { + final String id = randomAlphaOfLength(8); + final long retainingSequenceNumber = randomNonNegativeLong(); + final long timestamp = randomNonNegativeLong(); + final String source = randomAlphaOfLength(8); + final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, timestamp, source); + retentionLeases.add(retentionLease); + } + final RetentionLeases decodedRetentionLeases = + RetentionLeases.decodeRetentionLeases(RetentionLeases.encodeRetentionLeases(new RetentionLeases(version, retentionLeases))); + assertThat(decodedRetentionLeases.version(), equalTo(version)); + if (length == 0) { + assertThat(decodedRetentionLeases.retentionLeases(), empty()); + } else { + assertThat(decodedRetentionLeases.retentionLeases(), contains(retentionLeases.toArray(new RetentionLease[0]))); + } + } + +} \ No newline at end of file From 6ab494de6ead2d6b7a623933ccd009c6edf92965 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 13:10:40 -0500 Subject: [PATCH 05/26] Add primary term --- .../index/seqno/ReplicationTracker.java | 54 ++++++----- .../index/seqno/RetentionLease.java | 10 -- .../index/seqno/RetentionLeases.java | 96 ++++++++++++++++--- .../index/seqno/RetentionLeaseStatsTests.java | 2 +- .../index/seqno/RetentionLeaseSyncIT.java | 6 +- 5 files changed, 115 insertions(+), 53 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index 4aa79defff130..bcc7eca1c6ff8 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -28,6 +28,7 @@ import org.elasticsearch.cluster.routing.IndexShardRoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.collect.CopyOnWriteHashMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -38,9 +39,7 @@ import org.elasticsearch.index.shard.ShardId; import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -54,6 +53,7 @@ import java.util.function.ToLongFunction; import java.util.stream.Collectors; import java.util.stream.LongStream; +import java.util.stream.Stream; /** * This class is responsible for tracking the replication group with its progress and safety markers (local and global checkpoints). @@ -170,13 +170,7 @@ public class ReplicationTracker extends AbstractIndexShardComponent implements L */ volatile ReplicationGroup replicationGroup; - private volatile long version = 0; - private final Map retentionLeases = new HashMap<>(); - - private RetentionLeases copyRetentionLeases() { - assert Thread.holdsLock(this); - return new RetentionLeases(version, Collections.unmodifiableCollection(new ArrayList<>(retentionLeases.values()))); - } + private RetentionLeases retentionLeases = RetentionLeases.EMPTY; /** * Get all non-expired retention leases tracked on this shard. An unmodifiable copy of the retention leases is returned. Note that only @@ -192,20 +186,23 @@ public RetentionLeases getRetentionLeases() { // the primary calculates the non-expired retention leases and syncs them to replicas final long currentTimeMillis = currentTimeMillisSupplier.getAsLong(); final long retentionLeaseMillis = indexSettings.getRetentionLeaseMillis(); - final Collection expiredRetentionLeases = retentionLeases + + final Map leases = RetentionLeases.toMap(retentionLeases); + + final Collection expiredRetentionLeases = leases .values() .stream() .filter(retentionLease -> currentTimeMillis - retentionLease.timestamp() > retentionLeaseMillis) .collect(Collectors.toList()); if (expiredRetentionLeases.isEmpty()) { // early out as no retention leases have expired - return copyRetentionLeases(); + return retentionLeases; } // clean up the expired retention leases for (final RetentionLease expiredRetentionLease : expiredRetentionLeases) { - retentionLeases.remove(expiredRetentionLease.id()); + leases.remove(expiredRetentionLease.id()); } - version++; + retentionLeases = new RetentionLeases(operationPrimaryTerm, retentionLeases.version() + 1, leases.values()); } /* * At this point, we were either in primary mode and have updated the non-expired retention leases into the tracking map, or @@ -213,7 +210,7 @@ public RetentionLeases getRetentionLeases() { * non-expired retention leases, instead receiving them on syncs from the primary. */ wasPrimaryMode = primaryMode; - nonExpiredRetentionLeases = copyRetentionLeases(); + nonExpiredRetentionLeases = retentionLeases; } if (wasPrimaryMode) { onSyncRetentionLeases.accept(nonExpiredRetentionLeases, ActionListener.wrap(() -> {})); @@ -241,13 +238,15 @@ public RetentionLease addRetentionLease( final RetentionLeases currentRetentionLeases; synchronized (this) { assert primaryMode; - if (retentionLeases.containsKey(id)) { + if (retentionLeases.contains(id)) { throw new IllegalArgumentException("retention lease with ID [" + id + "] already exists"); } retentionLease = new RetentionLease(id, retainingSequenceNumber, currentTimeMillisSupplier.getAsLong(), source); - retentionLeases.put(id, retentionLease); - version++; - currentRetentionLeases = copyRetentionLeases(); + retentionLeases = new RetentionLeases( + operationPrimaryTerm, + retentionLeases.version() + 1, + Stream.concat(retentionLeases.leases().stream(), Stream.of(retentionLease)).collect(Collectors.toList())); + currentRetentionLeases = retentionLeases; } onSyncRetentionLeases.accept(currentRetentionLeases, listener); return retentionLease; @@ -264,19 +263,25 @@ public RetentionLease addRetentionLease( */ public synchronized RetentionLease renewRetentionLease(final String id, final long retainingSequenceNumber, final String source) { assert primaryMode; - if (retentionLeases.containsKey(id) == false) { + if (retentionLeases.contains(id) == false) { throw new IllegalArgumentException("retention lease with ID [" + id + "] does not exist"); } final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, currentTimeMillisSupplier.getAsLong(), source); - final RetentionLease existingRetentionLease = retentionLeases.put(id, retentionLease); - version++; + final RetentionLease existingRetentionLease = retentionLeases.get(id); assert existingRetentionLease != null; assert existingRetentionLease.retainingSequenceNumber() <= retentionLease.retainingSequenceNumber() : "retention lease renewal for [" + id + "]" + " from [" + source + "]" + " renewed a lower retaining sequence number [" + retentionLease.retainingSequenceNumber() + "]" + " than the current lease retaining sequence number [" + existingRetentionLease.retainingSequenceNumber() + "]"; + retentionLeases = new RetentionLeases( + operationPrimaryTerm, + retentionLeases.version() + 1, + Stream.concat( + retentionLeases.leases().stream().filter(lease -> lease.id().equals(id) == false), + Stream.of(retentionLease)) + .collect(Collectors.toList())); return retentionLease; } @@ -287,11 +292,8 @@ public synchronized RetentionLease renewRetentionLease(final String id, final lo */ public synchronized void updateRetentionLeasesOnReplica(final RetentionLeases retentionLeases) { assert primaryMode == false; - if (retentionLeases.version() > version) { - this.retentionLeases.clear(); - this.retentionLeases.putAll( - retentionLeases.leases().stream().collect(Collectors.toMap(RetentionLease::id, Function.identity()))); - this.version = retentionLeases.version(); + if (retentionLeases.supercedes(this.retentionLeases)) { + this.retentionLeases = retentionLeases; } } diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java index 4503b5e698d7d..258c9df698315 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java @@ -214,14 +214,4 @@ public String toString() { '}'; } - /** - * A utility method to convert a retention lease collection to a map from retention lease ID to retention lease. - * - * @param retentionLeases the retention lease collection - * @return the map from retention lease ID to retention lease - */ - static Map toMap(final RetentionLeases retentionLeases) { - return retentionLeases.leases().stream().collect(Collectors.toMap(RetentionLease::id, Function.identity())); - } - } diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 804a3879490f9..406aa0b2c47ee 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -24,12 +24,13 @@ import org.elasticsearch.common.io.stream.Writeable; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Locale; +import java.util.Map; import java.util.Objects; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -38,6 +39,17 @@ */ public class RetentionLeases implements Writeable { + private final long primaryTerm; + + /** + * The primary term of this retention lease collection. + * + * @return the primary term + */ + public long primaryTerm() { + return primaryTerm; + } + private final long version; /** @@ -50,7 +62,11 @@ public long version() { return version; } - private final Collection leases; + public boolean supercedes(final RetentionLeases that) { + return primaryTerm() > that.primaryTerm() || primaryTerm() <= that.primaryTerm() && version() > that.version(); + } + + private final Map leases; /** * The underlying collection of retention leases @@ -58,7 +74,27 @@ public long version() { * @return the retention leases */ public Collection leases() { - return leases; + return Collections.unmodifiableCollection(leases.values()); + } + + /** + * Checks if this retention lease collection contains a retention lease with the specified {@link RetentionLease#id()}. + * + * @param id the retention lease ID + * @return true if this retention lease collection contains a retention lease with the specified ID, otherwise false + */ + public boolean contains(final String id) { + return leases.containsKey(id); + } + + /** + * Returns the retention lease with the specified ID, or null if no such retention lease exists. + * + * @param id the retention lease ID + * @return the retention lease, or null if no retention lease with the specified ID exists + */ + public RetentionLease get(final String id) { + return leases.get(id); } /** @@ -74,12 +110,20 @@ public Collection leases() { * @param leases the retention leases */ public RetentionLeases(final long version, final Collection leases) { + this(0, version, leases); + } + + public RetentionLeases(final long primaryTerm, final long version, final Collection leases) { + if (primaryTerm < 0) { + throw new IllegalArgumentException("primary term must be non-negative but was [" + primaryTerm + "]"); + } if (version < 0) { throw new IllegalArgumentException("version must be non-negative but was [" + version + "]"); } Objects.requireNonNull(leases); + this.primaryTerm = primaryTerm; this.version = version; - this.leases = Collections.unmodifiableCollection(new ArrayList<>(leases)); + this.leases = Collections.unmodifiableMap(toMap(leases)); } /** @@ -90,8 +134,9 @@ public RetentionLeases(final long version, final Collection leas * @throws IOException if an I/O exception occurs reading from the stream */ public RetentionLeases(final StreamInput in) throws IOException { + primaryTerm = in.readVLong(); version = in.readVLong(); - leases = in.readList(RetentionLease::new); + leases = Collections.unmodifiableMap(toMap(in.readList(RetentionLease::new))); } /** @@ -103,8 +148,9 @@ public RetentionLeases(final StreamInput in) throws IOException { */ @Override public void writeTo(final StreamOutput out) throws IOException { + out.writeVLong(primaryTerm); out.writeVLong(version); - out.writeCollection(leases); + out.writeCollection(leases.values()); } /** @@ -119,7 +165,8 @@ public static String encodeRetentionLeases(final RetentionLeases retentionLeases Objects.requireNonNull(retentionLeases); return String.format( Locale.ROOT, - "version:%d;%s", + "primary_term:%d;version:%d;%s", + retentionLeases.primaryTerm(), retentionLeases.version(), retentionLeases.leases().stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(","))); } @@ -135,30 +182,53 @@ public static RetentionLeases decodeRetentionLeases(final String encodedRetentio if (encodedRetentionLeases.isEmpty()) { return EMPTY; } - assert encodedRetentionLeases.matches("version:\\d+;.*") : encodedRetentionLeases; + assert encodedRetentionLeases.matches("primary_term:\\d+;version:\\d+;.*") : encodedRetentionLeases; final int firstSemicolon = encodedRetentionLeases.indexOf(";"); - final long version = Long.parseLong(encodedRetentionLeases.substring("version:".length(), firstSemicolon)); + final long primaryTerm = Long.parseLong(encodedRetentionLeases.substring("primary_term:".length(), firstSemicolon)); + final int secondSemicolon = encodedRetentionLeases.indexOf(";", firstSemicolon + 1); + final long version = Long.parseLong(encodedRetentionLeases.substring(firstSemicolon + 1 + "version:".length(), secondSemicolon)); final Collection retentionLeases; if (firstSemicolon + 1 == encodedRetentionLeases.length()) { retentionLeases = Collections.emptyList(); } else { - assert Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) + assert Arrays.stream(encodedRetentionLeases.substring(secondSemicolon + 1).split(",")) .allMatch(s -> s.matches("id:[^:;,]+;retaining_seq_no:\\d+;timestamp:\\d+;source:[^:;,]+")) : encodedRetentionLeases; - retentionLeases = Arrays.stream(encodedRetentionLeases.substring(firstSemicolon + 1).split(",")) + retentionLeases = Arrays.stream(encodedRetentionLeases.substring(secondSemicolon + 1).split(",")) .map(RetentionLease::decodeRetentionLease) .collect(Collectors.toList()); } - return new RetentionLeases(version, retentionLeases); + return new RetentionLeases(primaryTerm, version, retentionLeases); } @Override public String toString() { return "RetentionLeases{" + - "version=" + version + + "primaryTerm=" + primaryTerm + + ", version=" + version + ", leases=" + leases + '}'; } + /** + * A utility method to convert retention leases to a map from retention lease ID to retention lease. + * + * @param leases the retention leases + * @return the map from retention lease ID to retention lease + */ + private static Map toMap(final Collection leases) { + return leases.stream().collect(Collectors.toMap(RetentionLease::id, Function.identity())); + } + + /** + * A utility method to convert a retention lease collection to a map from retention lease ID to retention lease. + * + * @param retentionLeases the retention lease collection + * @return the map from retention lease ID to retention lease + */ + static Map toMap(final RetentionLeases retentionLeases) { + return retentionLeases.leases; + } + } diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsTests.java index 573fdb3e3df1a..8721450073531 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsTests.java @@ -61,7 +61,7 @@ public void testRetentionLeaseStats() throws InterruptedException { final IndicesStatsResponse indicesStats = client().admin().indices().prepareStats("index").execute().actionGet(); assertThat(indicesStats.getShards(), arrayWithSize(1)); final RetentionLeaseStats retentionLeaseStats = indicesStats.getShards()[0].getRetentionLeaseStats(); - assertThat(RetentionLease.toMap(retentionLeaseStats.retentionLeases()), equalTo(currentRetentionLeases)); + assertThat(RetentionLeases.toMap(retentionLeaseStats.retentionLeases()), equalTo(currentRetentionLeases)); } } diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java index 6e19176b6bbce..e2b58427868fb 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java @@ -75,7 +75,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { // check retention leases have been committed on the primary final RetentionLeases primaryCommittedRetentionLeases = RetentionLeases.decodeRetentionLeases( primary.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES)); - assertThat(currentRetentionLeases, equalTo(RetentionLease.toMap(primaryCommittedRetentionLeases))); + assertThat(currentRetentionLeases, equalTo(RetentionLeases.toMap(primaryCommittedRetentionLeases))); // check current retention leases have been synced to all replicas for (final ShardRouting replicaShard : clusterService().state().routingTable().index("index").shard(0).replicaShards()) { @@ -84,13 +84,13 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception { final IndexShard replica = internalCluster() .getInstance(IndicesService.class, replicaShardNodeName) .getShardOrNull(new ShardId(resolveIndex("index"), 0)); - final Map retentionLeasesOnReplica = RetentionLease.toMap(replica.getRetentionLeases()); + final Map retentionLeasesOnReplica = RetentionLeases.toMap(replica.getRetentionLeases()); assertThat(retentionLeasesOnReplica, equalTo(currentRetentionLeases)); // check retention leases have been committed on the replica final RetentionLeases replicaCommittedRetentionLeases = RetentionLeases.decodeRetentionLeases( replica.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES)); - assertThat(currentRetentionLeases, equalTo(RetentionLease.toMap(replicaCommittedRetentionLeases))); + assertThat(currentRetentionLeases, equalTo(RetentionLeases.toMap(replicaCommittedRetentionLeases))); } } } From 954db8180ab4bb6880d536735f20ffd6b3bb116f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 13:23:49 -0500 Subject: [PATCH 06/26] Remove dead constructor --- .../index/seqno/RetentionLeases.java | 24 ++++++++++++++----- .../index/engine/InternalEngineTests.java | 5 ++-- .../index/engine/SoftDeletesPolicyTests.java | 2 +- ...ReplicationTrackerRetentionLeaseTests.java | 5 +++- ...tentionLeaseStatsWireSerializingTests.java | 3 ++- .../index/seqno/RetentionLeasesTests.java | 7 ++++-- .../shard/IndexShardRetentionLeaseTests.java | 2 ++ 7 files changed, 35 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 406aa0b2c47ee..7ae9bcddb0f22 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -101,18 +101,15 @@ public RetentionLease get(final String id) { * Represents an empty an un-versioned retention lease collection. This is used when no retention lease collection is found in the * commit point */ - public static RetentionLeases EMPTY = new RetentionLeases(0, Collections.emptyList()); + public static RetentionLeases EMPTY = new RetentionLeases(0, 0, Collections.emptyList()); /** * Constructs a new retention lease collection with the specified version and underlying collection of retention leases. * + * @param primaryTerm the primary term under which this retention lease collection was created * @param version the version of this retention lease collection * @param leases the retention leases */ - public RetentionLeases(final long version, final Collection leases) { - this(0, version, leases); - } - public RetentionLeases(final long primaryTerm, final long version, final Collection leases) { if (primaryTerm < 0) { throw new IllegalArgumentException("primary term must be non-negative but was [" + primaryTerm + "]"); @@ -188,7 +185,7 @@ public static RetentionLeases decodeRetentionLeases(final String encodedRetentio final int secondSemicolon = encodedRetentionLeases.indexOf(";", firstSemicolon + 1); final long version = Long.parseLong(encodedRetentionLeases.substring(firstSemicolon + 1 + "version:".length(), secondSemicolon)); final Collection retentionLeases; - if (firstSemicolon + 1 == encodedRetentionLeases.length()) { + if (secondSemicolon + 1 == encodedRetentionLeases.length()) { retentionLeases = Collections.emptyList(); } else { assert Arrays.stream(encodedRetentionLeases.substring(secondSemicolon + 1).split(",")) @@ -202,6 +199,21 @@ public static RetentionLeases decodeRetentionLeases(final String encodedRetentio return new RetentionLeases(primaryTerm, version, retentionLeases); } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final RetentionLeases that = (RetentionLeases) o; + return primaryTerm == that.primaryTerm && + version == that.version && + Objects.equals(leases, that.leases); + } + + @Override + public int hashCode() { + return Objects.hash(primaryTerm, version, leases); + } + @Override public String toString() { return "RetentionLeases{" + diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index dfa5672c9a583..017a865d74999 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -5304,7 +5304,8 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { final IndexMetaData indexMetaData = IndexMetaData.builder(defaultSettings.getIndexMetaData()).settings(settings).build(); final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(indexMetaData); final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); - final AtomicLong retentionLeasesVersion = new AtomicLong(0); + final long primaryTerm = randomLongBetween(1, Long.MAX_VALUE); + final AtomicLong retentionLeasesVersion = new AtomicLong(); final AtomicReference leasesHolder = new AtomicReference<>(RetentionLeases.EMPTY); final List operations = generateSingleDocHistory(true, randomFrom(VersionType.INTERNAL, VersionType.EXTERNAL), 2, 10, 300, "2"); @@ -5337,7 +5338,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { final String source = randomAlphaOfLength(8); leases.add(new RetentionLease(id, retainingSequenceNumber, timestamp, source)); } - leasesHolder.set(new RetentionLeases(retentionLeasesVersion.get(), leases)); + leasesHolder.set(new RetentionLeases(primaryTerm, retentionLeasesVersion.get(), leases)); } if (rarely()) { settings.put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING.getKey(), randomLongBetween(0, 10)); diff --git a/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java b/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java index ce103c3725b40..29c0fd3e9931a 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java @@ -59,7 +59,7 @@ public void testSoftDeletesRetentionLock() { for (int i = 0; i < retainingSequenceNumbers.length; i++) { leases.add(new RetentionLease(Integer.toString(i), retainingSequenceNumbers[i].get(), 0L, "test")); } - return new RetentionLeases(1, leases); + return new RetentionLeases(1, 1, leases); }; long safeCommitCheckpoint = globalCheckpoint.get(); SoftDeletesPolicy policy = new SoftDeletesPolicy(globalCheckpoint::get, between(1, 10000), retainedOps, retentionLeasesSupplier); diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index 3c7956b3be66f..f84f1d83a2b70 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -169,6 +169,7 @@ private void runExpirationTest(final boolean primaryMode) { replicationTracker.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {})); } else { final RetentionLeases retentionLeases = new RetentionLeases( + 1, 1, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); @@ -190,6 +191,7 @@ private void runExpirationTest(final boolean primaryMode) { replicationTracker.renewRetentionLease("0", retainingSequenceNumbers[0], "test-0"); } else { final RetentionLeases retentionLeases = new RetentionLeases( + 1, 2, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); @@ -301,6 +303,7 @@ public void testRetentionLeaseExpirationCausesRetentionLeaseSync() { assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); } + // TODO: update this test public void testReplicaIgnoresOlderRetentionLeasesVersion() { final AllocationId allocationId = AllocationId.newInitializing(); final ReplicationTracker replicationTracker = new ReplicationTracker( @@ -328,7 +331,7 @@ public void testReplicaIgnoresOlderRetentionLeasesVersion() { new RetentionLease(i + "-" + j, randomNonNegativeLong(), randomNonNegativeLong(), randomAlphaOfLength(8))); version++; } - retentionLeasesCollection.add(new RetentionLeases(version, retentionLeases)); + retentionLeasesCollection.add(new RetentionLeases(1, version, retentionLeases)); } Collections.shuffle(retentionLeasesCollection, random()); diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsWireSerializingTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsWireSerializingTests.java index 0ed709f186dba..9c7aee5191ac8 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsWireSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseStatsWireSerializingTests.java @@ -30,6 +30,7 @@ public class RetentionLeaseStatsWireSerializingTests extends AbstractWireSeriali @Override protected RetentionLeaseStats createTestInstance() { + final long primaryTerm = randomNonNegativeLong(); final long version = randomNonNegativeLong(); final int length = randomIntBetween(0, 8); final Collection leases; @@ -45,7 +46,7 @@ protected RetentionLeaseStats createTestInstance() { leases.add(new RetentionLease(id, retainingSequenceNumber, timestamp, source)); } } - return new RetentionLeaseStats(new RetentionLeases(version, leases)); + return new RetentionLeaseStats(new RetentionLeases(primaryTerm, version, leases)); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java index cbfb3972f913d..daa255bb09a26 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java @@ -25,12 +25,14 @@ import java.util.List; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; public class RetentionLeasesTests extends ESTestCase { public void testRetentionLeasesEncoding() { + final long primaryTerm = randomNonNegativeLong(); final long version = randomNonNegativeLong(); final int length = randomIntBetween(0, 8); final List retentionLeases = new ArrayList<>(length); @@ -43,12 +45,13 @@ public void testRetentionLeasesEncoding() { retentionLeases.add(retentionLease); } final RetentionLeases decodedRetentionLeases = - RetentionLeases.decodeRetentionLeases(RetentionLeases.encodeRetentionLeases(new RetentionLeases(version, retentionLeases))); + RetentionLeases.decodeRetentionLeases( + RetentionLeases.encodeRetentionLeases(new RetentionLeases(primaryTerm, version, retentionLeases))); assertThat(decodedRetentionLeases.version(), equalTo(version)); if (length == 0) { assertThat(decodedRetentionLeases.leases(), empty()); } else { - assertThat(decodedRetentionLeases.leases(), contains(retentionLeases.toArray(new RetentionLease[0]))); + assertThat(decodedRetentionLeases.leases(), containsInAnyOrder(retentionLeases.toArray(new RetentionLease[0]))); } } diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java index 795e49885cf63..4d8ddea447a64 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java @@ -120,6 +120,7 @@ private void runExpirationTest(final boolean primary) throws IOException { })); } else { final RetentionLeases retentionLeases = new RetentionLeases( + 1, 1, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); indexShard.updateRetentionLeasesOnReplica(retentionLeases); @@ -141,6 +142,7 @@ private void runExpirationTest(final boolean primary) throws IOException { indexShard.renewRetentionLease("0", retainingSequenceNumbers[0], "test-0"); } else { final RetentionLeases retentionLeases = new RetentionLeases( + 1, 2, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); indexShard.updateRetentionLeasesOnReplica(retentionLeases); From 9d0998962f3f0b47879ce212ab65e7b191320381 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 16:33:37 -0500 Subject: [PATCH 07/26] Partition leases when calculating expiration --- .../index/seqno/ReplicationTracker.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index bcc7eca1c6ff8..36b62ae282a93 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -40,8 +40,10 @@ import java.io.IOException; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.OptionalLong; @@ -186,23 +188,18 @@ public RetentionLeases getRetentionLeases() { // the primary calculates the non-expired retention leases and syncs them to replicas final long currentTimeMillis = currentTimeMillisSupplier.getAsLong(); final long retentionLeaseMillis = indexSettings.getRetentionLeaseMillis(); - final Map leases = RetentionLeases.toMap(retentionLeases); - - final Collection expiredRetentionLeases = leases + final Map> partition = leases .values() .stream() - .filter(retentionLease -> currentTimeMillis - retentionLease.timestamp() > retentionLeaseMillis) - .collect(Collectors.toList()); - if (expiredRetentionLeases.isEmpty()) { + .collect(Collectors.groupingBy(lease -> currentTimeMillis - lease.timestamp() > retentionLeaseMillis)); + if (partition.get(true) == null) { // early out as no retention leases have expired return retentionLeases; } - // clean up the expired retention leases - for (final RetentionLease expiredRetentionLease : expiredRetentionLeases) { - leases.remove(expiredRetentionLease.id()); - } - retentionLeases = new RetentionLeases(operationPrimaryTerm, retentionLeases.version() + 1, leases.values()); + final Collection nonExpiredLeases = + partition.get(false) != null ? partition.get(false) : Collections.emptyList(); + retentionLeases = new RetentionLeases(operationPrimaryTerm, retentionLeases.version() + 1, nonExpiredLeases); } /* * At this point, we were either in primary mode and have updated the non-expired retention leases into the tracking map, or From 17d2095ec76f0d1696c27d13a3fcf5088474861b Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 16:35:04 -0500 Subject: [PATCH 08/26] Remove unnecessary map --- .../org/elasticsearch/index/seqno/ReplicationTracker.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index 36b62ae282a93..e7b01e7577534 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -188,9 +188,8 @@ public RetentionLeases getRetentionLeases() { // the primary calculates the non-expired retention leases and syncs them to replicas final long currentTimeMillis = currentTimeMillisSupplier.getAsLong(); final long retentionLeaseMillis = indexSettings.getRetentionLeaseMillis(); - final Map leases = RetentionLeases.toMap(retentionLeases); - final Map> partition = leases - .values() + final Map> partition = retentionLeases + .leases() .stream() .collect(Collectors.groupingBy(lease -> currentTimeMillis - lease.timestamp() > retentionLeaseMillis)); if (partition.get(true) == null) { From e724746aeaa9e1e697ef2f767d0cf31077ebc6e4 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 16:52:39 -0500 Subject: [PATCH 09/26] Fix test --- ...ReplicationTrackerRetentionLeaseTests.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index f84f1d83a2b70..ae366589a1509 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.IndexSettingsModule; +import java.lang.annotation.Retention; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -303,7 +304,6 @@ public void testRetentionLeaseExpirationCausesRetentionLeaseSync() { assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); } - // TODO: update this test public void testReplicaIgnoresOlderRetentionLeasesVersion() { final AllocationId allocationId = AllocationId.newInitializing(); final ReplicationTracker replicationTracker = new ReplicationTracker( @@ -322,6 +322,7 @@ public void testReplicaIgnoresOlderRetentionLeasesVersion() { Collections.emptySet()); final int length = randomIntBetween(0, 8); final List retentionLeasesCollection = new ArrayList<>(length); + long primaryTerm = 1; long version = 0; for (int i = 0; i < length; i++) { final int innerLength = randomIntBetween(0, 8); @@ -331,21 +332,28 @@ public void testReplicaIgnoresOlderRetentionLeasesVersion() { new RetentionLease(i + "-" + j, randomNonNegativeLong(), randomNonNegativeLong(), randomAlphaOfLength(8))); version++; } - retentionLeasesCollection.add(new RetentionLeases(1, version, retentionLeases)); + if (rarely()) { + primaryTerm++; + } + retentionLeasesCollection.add(new RetentionLeases(primaryTerm, version, retentionLeases)); + } + final Collection expectedLeases; + if (length == 0 || retentionLeasesCollection.get(length - 1).leases().isEmpty()) { + expectedLeases = Collections.emptyList(); + } else { + expectedLeases = retentionLeasesCollection.get(length - 1).leases(); } - Collections.shuffle(retentionLeasesCollection, random()); for (final RetentionLeases retentionLeases : retentionLeasesCollection) { replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); } assertThat(replicationTracker.getRetentionLeases().version(), equalTo(version)); - if (length == 0) { + if (expectedLeases.isEmpty()) { assertThat(replicationTracker.getRetentionLeases().leases(), empty()); } else { - final RetentionLeases expected = retentionLeasesCollection.get(length - 1); assertThat( replicationTracker.getRetentionLeases().leases(), - contains(expected.leases().toArray(new RetentionLease[0]))); + contains(expectedLeases.toArray(new RetentionLease[0]))); } } From e8971b9fa90391cdc62aff3bc03ae199b2a353bc Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 16:55:18 -0500 Subject: [PATCH 10/26] Fix spelling and imports --- .../java/org/elasticsearch/index/seqno/ReplicationTracker.java | 3 +-- .../java/org/elasticsearch/index/seqno/RetentionLeases.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index e7b01e7577534..74d76afe28c0d 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -28,7 +28,6 @@ import org.elasticsearch.cluster.routing.IndexShardRoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.collect.CopyOnWriteHashMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -288,7 +287,7 @@ public synchronized RetentionLease renewRetentionLease(final String id, final lo */ public synchronized void updateRetentionLeasesOnReplica(final RetentionLeases retentionLeases) { assert primaryMode == false; - if (retentionLeases.supercedes(this.retentionLeases)) { + if (retentionLeases.supersedes(this.retentionLeases)) { this.retentionLeases = retentionLeases; } } diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 7ae9bcddb0f22..d298d2205cf14 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -62,7 +62,7 @@ public long version() { return version; } - public boolean supercedes(final RetentionLeases that) { + public boolean supersedes(final RetentionLeases that) { return primaryTerm() > that.primaryTerm() || primaryTerm() <= that.primaryTerm() && version() > that.version(); } From f3cb4eb437a5f4f8d2d7bd1c6dd9f2ad3ab288c6 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 17:06:28 -0500 Subject: [PATCH 11/26] Add tests and docs for supersedes --- .../index/seqno/RetentionLeases.java | 9 ++++++++- .../index/seqno/RetentionLeasesTests.java | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index d298d2205cf14..a4b9491227c36 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -62,8 +62,15 @@ public long version() { return version; } + /** + * Checks if this retention leases collection supersedes the specified retention leases collection. A retention leases collection + * supersedes another retention leases collection if its primary term is higher, or if for equal primary terms its version is higher + * + * @param that the retention leases collection to test against + * @return true if this retention leases collection supercedes the specified retention lease collection, otherwise false + */ public boolean supersedes(final RetentionLeases that) { - return primaryTerm() > that.primaryTerm() || primaryTerm() <= that.primaryTerm() && version() > that.version(); + return primaryTerm() > that.primaryTerm() || primaryTerm() == that.primaryTerm() && version() > that.version(); } private final Map leases; diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java index daa255bb09a26..544fb8d2c2bd4 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import static org.hamcrest.Matchers.contains; @@ -55,4 +56,23 @@ public void testRetentionLeasesEncoding() { } } + public void testSupersedesByPrimaryTerm() { + final long lowerPrimaryTerm = randomLongBetween(1, Long.MAX_VALUE); + final RetentionLeases left = new RetentionLeases(lowerPrimaryTerm, randomLongBetween(1, Long.MAX_VALUE), Collections.emptyList()); + final long higherPrimaryTerm = randomLongBetween(lowerPrimaryTerm + 1, Long.MAX_VALUE); + final RetentionLeases right = new RetentionLeases(higherPrimaryTerm, randomLongBetween(1, Long.MAX_VALUE), Collections.emptyList()); + assertTrue(right.supersedes(left)); + assertFalse(left.supersedes(right)); + } + + public void testSupersedesByVersion() { + final long primaryTerm = randomLongBetween(1, Long.MAX_VALUE); + final long lowerVersion = randomLongBetween(1, Long.MAX_VALUE); + final long higherVersion = randomLongBetween(lowerVersion + 1, Long.MAX_VALUE); + final RetentionLeases left = new RetentionLeases(primaryTerm, lowerVersion, Collections.emptyList()); + final RetentionLeases right = new RetentionLeases(primaryTerm, higherVersion, Collections.emptyList()); + assertTrue(right.supersedes(left)); + assertFalse(left.supersedes(right)); + } + } \ No newline at end of file From 0a7d19b657568b5148aecf96a6e153d5b9020de1 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 17:10:57 -0500 Subject: [PATCH 12/26] Adjust primary term limit --- .../index/seqno/RetentionLeases.java | 6 +++--- .../index/seqno/RetentionLeasesTests.java | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index a4b9491227c36..01ce3ec0ff03f 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -108,7 +108,7 @@ public RetentionLease get(final String id) { * Represents an empty an un-versioned retention lease collection. This is used when no retention lease collection is found in the * commit point */ - public static RetentionLeases EMPTY = new RetentionLeases(0, 0, Collections.emptyList()); + public static RetentionLeases EMPTY = new RetentionLeases(1, 0, Collections.emptyList()); /** * Constructs a new retention lease collection with the specified version and underlying collection of retention leases. @@ -118,8 +118,8 @@ public RetentionLease get(final String id) { * @param leases the retention leases */ public RetentionLeases(final long primaryTerm, final long version, final Collection leases) { - if (primaryTerm < 0) { - throw new IllegalArgumentException("primary term must be non-negative but was [" + primaryTerm + "]"); + if (primaryTerm <= 0) { + throw new IllegalArgumentException("primary term must be positive but was [" + primaryTerm + "]"); } if (version < 0) { throw new IllegalArgumentException("version must be non-negative but was [" + version + "]"); diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java index 544fb8d2c2bd4..c4359b4e8697d 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java @@ -27,11 +27,29 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasToString; public class RetentionLeasesTests extends ESTestCase { + public void testPrimaryTermOutOfRange() { + final long primaryTerm = randomLongBetween(Long.MIN_VALUE, 0); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new RetentionLeases(primaryTerm, randomNonNegativeLong(), Collections.emptyList())); + assertThat(e, hasToString(containsString("primary term must be positive but was [" + primaryTerm + "]"))); + } + + public void testVersionOutOfRange() { + final long version = randomLongBetween(Long.MIN_VALUE, -1); + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new RetentionLeases(randomLongBetween(1, Long.MAX_VALUE), version, Collections.emptyList())); + assertThat(e, hasToString(containsString("version must be non-negative but was [" + version + "]"))); + } + public void testRetentionLeasesEncoding() { final long primaryTerm = randomNonNegativeLong(); final long version = randomNonNegativeLong(); From 034a8402551bead92382c3edab0a261dce6e8689 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 17:13:18 -0500 Subject: [PATCH 13/26] Add missing newline --- .../org/elasticsearch/index/seqno/RetentionLeasesTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java index c4359b4e8697d..bc6b39c8d8814 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java @@ -93,4 +93,4 @@ public void testSupersedesByVersion() { assertFalse(left.supersedes(right)); } -} \ No newline at end of file +} From 05b69c01c53e1e3c0232559a138dc3f3f4f43581 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 17:25:43 -0500 Subject: [PATCH 14/26] Update tests --- .../index/seqno/ReplicationTracker.java | 3 ++ ...ReplicationTrackerRetentionLeaseTests.java | 32 +++++++++++++------ .../shard/IndexShardRetentionLeaseTests.java | 24 +++++++++----- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index 74d76afe28c0d..1af1033f5dcfa 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -171,6 +171,9 @@ public class ReplicationTracker extends AbstractIndexShardComponent implements L */ volatile ReplicationGroup replicationGroup; + /** + * The current retention leases. + */ private RetentionLeases retentionLeases = RetentionLeases.EMPTY; /** diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index ae366589a1509..6c7c01e89db7e 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -53,11 +53,12 @@ public class ReplicationTrackerRetentionLeaseTests extends ReplicationTrackerTes public void testAddOrRenewRetentionLease() { final AllocationId allocationId = AllocationId.newInitializing(); + long primaryTerm = randomLongBetween(1, Long.MAX_VALUE); final ReplicationTracker replicationTracker = new ReplicationTracker( new ShardId("test", "_na", 0), allocationId.getId(), IndexSettingsModule.newIndexSettings("test", Settings.EMPTY), - randomNonNegativeLong(), + primaryTerm, UNASSIGNED_SEQ_NO, value -> {}, () -> 0L, @@ -74,13 +75,21 @@ public void testAddOrRenewRetentionLease() { minimumRetainingSequenceNumbers[i] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE); replicationTracker.addRetentionLease( Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i, ActionListener.wrap(() -> {})); - assertRetentionLeases(replicationTracker, i + 1, minimumRetainingSequenceNumbers, () -> 0L, 1 + i, true); + if (rarely() && primaryTerm < Long.MAX_VALUE) { + primaryTerm = randomLongBetween(primaryTerm + 1, Long.MAX_VALUE); + replicationTracker.setOperationPrimaryTerm(primaryTerm); + } + assertRetentionLeases(replicationTracker, i + 1, minimumRetainingSequenceNumbers, () -> 0L, primaryTerm, 1 + i, true); } for (int i = 0; i < length; i++) { minimumRetainingSequenceNumbers[i] = randomLongBetween(minimumRetainingSequenceNumbers[i], Long.MAX_VALUE); replicationTracker.renewRetentionLease(Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i); - assertRetentionLeases(replicationTracker, length, minimumRetainingSequenceNumbers, () -> 0L, 1 + length + i, true); + if (rarely() && primaryTerm < Long.MAX_VALUE) { + primaryTerm = randomLongBetween(primaryTerm + 1, Long.MAX_VALUE); + replicationTracker.setOperationPrimaryTerm(primaryTerm); + } + assertRetentionLeases(replicationTracker, length, minimumRetainingSequenceNumbers, () -> 0L, primaryTerm, 1 + length + i, true); } } @@ -147,11 +156,12 @@ private void runExpirationTest(final boolean primaryMode) { .builder() .put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_LEASE_SETTING.getKey(), TimeValue.timeValueMillis(retentionLeaseMillis)) .build(); + final long primaryTerm = randomLongBetween(1, Long.MAX_VALUE); final ReplicationTracker replicationTracker = new ReplicationTracker( new ShardId("test", "_na", 0), allocationId.getId(), IndexSettingsModule.newIndexSettings("test", settings), - randomNonNegativeLong(), + primaryTerm, UNASSIGNED_SEQ_NO, value -> {}, currentTimeMillis::get, @@ -170,7 +180,7 @@ private void runExpirationTest(final boolean primaryMode) { replicationTracker.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {})); } else { final RetentionLeases retentionLeases = new RetentionLeases( - 1, + primaryTerm, 1, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); @@ -182,7 +192,7 @@ private void runExpirationTest(final boolean primaryMode) { assertThat(retentionLeases.leases(), hasSize(1)); final RetentionLease retentionLease = retentionLeases.leases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, 1, primaryMode); + assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 1, primaryMode); } // renew the lease @@ -192,7 +202,7 @@ private void runExpirationTest(final boolean primaryMode) { replicationTracker.renewRetentionLease("0", retainingSequenceNumbers[0], "test-0"); } else { final RetentionLeases retentionLeases = new RetentionLeases( - 1, + primaryTerm, 2, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); replicationTracker.updateRetentionLeasesOnReplica(retentionLeases); @@ -204,16 +214,16 @@ private void runExpirationTest(final boolean primaryMode) { assertThat(retentionLeases.leases(), hasSize(1)); final RetentionLease retentionLease = retentionLeases.leases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, primaryMode); + assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 2, primaryMode); } // now force the lease to expire currentTimeMillis.set(currentTimeMillis.get() + randomLongBetween(retentionLeaseMillis, Long.MAX_VALUE - currentTimeMillis.get())); if (primaryMode) { - assertRetentionLeases(replicationTracker, 0, retainingSequenceNumbers, currentTimeMillis::get, 3, true); + assertRetentionLeases(replicationTracker, 0, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 3, true); } else { // leases do not expire on replicas until synced from the primary - assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, false); + assertRetentionLeases(replicationTracker, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 2, false); } } @@ -366,9 +376,11 @@ private void assertRetentionLeases( final int size, final long[] minimumRetainingSequenceNumbers, final LongSupplier currentTimeMillisSupplier, + final long primaryTerm, final long version, final boolean primaryMode) { final RetentionLeases retentionLeases = replicationTracker.getRetentionLeases(); + assertThat(retentionLeases.primaryTerm(), equalTo(primaryTerm)); assertThat(retentionLeases.version(), equalTo(version)); final Map idToRetentionLease = new HashMap<>(); for (final RetentionLease retentionLease : retentionLeases.leases()) { diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java index 4d8ddea447a64..1d7f81adde5b9 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java @@ -76,6 +76,7 @@ protected void tearDownThreadPool() { public void testAddOrRenewRetentionLease() throws IOException { final IndexShard indexShard = newStartedShard(true); + final long primaryTerm = indexShard.getOperationPrimaryTerm(); try { final int length = randomIntBetween(0, 8); final long[] minimumRetainingSequenceNumbers = new long[length]; @@ -83,13 +84,14 @@ public void testAddOrRenewRetentionLease() throws IOException { minimumRetainingSequenceNumbers[i] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE); indexShard.addRetentionLease( Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i, ActionListener.wrap(() -> {})); - assertRetentionLeases(indexShard, i + 1, minimumRetainingSequenceNumbers, () -> 0L, 1 + i, true); + assertRetentionLeases( + indexShard, i + 1, minimumRetainingSequenceNumbers, () -> 0L, primaryTerm, 1 + i, true); } for (int i = 0; i < length; i++) { minimumRetainingSequenceNumbers[i] = randomLongBetween(minimumRetainingSequenceNumbers[i], Long.MAX_VALUE); indexShard.renewRetentionLease(Integer.toString(i), minimumRetainingSequenceNumbers[i], "test-" + i); - assertRetentionLeases(indexShard, length, minimumRetainingSequenceNumbers, () -> 0L, 1 + length + i, true); + assertRetentionLeases(indexShard, length, minimumRetainingSequenceNumbers, () -> 0L, primaryTerm, 1 + length + i, true); } } finally { closeShards(indexShard); @@ -112,6 +114,7 @@ private void runExpirationTest(final boolean primary) throws IOException { .build(); // current time is mocked through the thread pool final IndexShard indexShard = newStartedShard(primary, settings, new InternalEngineFactory()); + final long primaryTerm = indexShard.getOperationPrimaryTerm(); try { final long[] retainingSequenceNumbers = new long[1]; retainingSequenceNumbers[0] = randomLongBetween(0, Long.MAX_VALUE); @@ -120,7 +123,7 @@ private void runExpirationTest(final boolean primary) throws IOException { })); } else { final RetentionLeases retentionLeases = new RetentionLeases( - 1, + primaryTerm, 1, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); indexShard.updateRetentionLeasesOnReplica(retentionLeases); @@ -132,7 +135,7 @@ private void runExpirationTest(final boolean primary) throws IOException { assertThat(retentionLeases.leases(), hasSize(1)); final RetentionLease retentionLease = retentionLeases.leases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, 1, primary); + assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 1, primary); } // renew the lease @@ -142,7 +145,7 @@ private void runExpirationTest(final boolean primary) throws IOException { indexShard.renewRetentionLease("0", retainingSequenceNumbers[0], "test-0"); } else { final RetentionLeases retentionLeases = new RetentionLeases( - 1, + primaryTerm, 2, Collections.singleton(new RetentionLease("0", retainingSequenceNumbers[0], currentTimeMillis.get(), "test-0"))); indexShard.updateRetentionLeasesOnReplica(retentionLeases); @@ -154,16 +157,16 @@ private void runExpirationTest(final boolean primary) throws IOException { assertThat(retentionLeases.leases(), hasSize(1)); final RetentionLease retentionLease = retentionLeases.leases().iterator().next(); assertThat(retentionLease.timestamp(), equalTo(currentTimeMillis.get())); - assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, primary); + assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 2, primary); } // now force the lease to expire currentTimeMillis.set( currentTimeMillis.get() + randomLongBetween(retentionLeaseMillis, Long.MAX_VALUE - currentTimeMillis.get())); if (primary) { - assertRetentionLeases(indexShard, 0, retainingSequenceNumbers, currentTimeMillis::get, 3, true); + assertRetentionLeases(indexShard, 0, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 3, true); } else { - assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, 2, false); + assertRetentionLeases(indexShard, 1, retainingSequenceNumbers, currentTimeMillis::get, primaryTerm, 2, false); } } finally { closeShards(indexShard); @@ -249,6 +252,7 @@ public void testRetentionLeaseStats() throws IOException { length, minimumRetainingSequenceNumbers, () -> 0L, + indexShard.getOperationPrimaryTerm(), length, true); } finally { @@ -261,6 +265,7 @@ private void assertRetentionLeases( final int size, final long[] minimumRetainingSequenceNumbers, final LongSupplier currentTimeMillisSupplier, + final long primaryTerm, final long version, final boolean primary) { assertRetentionLeases( @@ -269,6 +274,7 @@ private void assertRetentionLeases( size, minimumRetainingSequenceNumbers, currentTimeMillisSupplier, + primaryTerm, version, primary); } @@ -279,8 +285,10 @@ private void assertRetentionLeases( final int size, final long[] minimumRetainingSequenceNumbers, final LongSupplier currentTimeMillisSupplier, + final long primaryTerm, final long version, final boolean primary) { + assertThat(retentionLeases.primaryTerm(), equalTo(primaryTerm)); assertThat(retentionLeases.version(), equalTo(version)); final Map idToRetentionLease = new HashMap<>(); for (final RetentionLease retentionLease : retentionLeases.leases()) { From 600809cf0f41c4adc9470db83f0bd544d9d8548b Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 17:59:58 -0500 Subject: [PATCH 15/26] Fix test --- .../index/shard/IndexShardRetentionLeaseTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java index 1d7f81adde5b9..76ca9f5b02458 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java @@ -252,7 +252,7 @@ public void testRetentionLeaseStats() throws IOException { length, minimumRetainingSequenceNumbers, () -> 0L, - indexShard.getOperationPrimaryTerm(), + length == 0 ? RetentionLeases.EMPTY.primaryTerm() : indexShard.getOperationPrimaryTerm(), length, true); } finally { From 39023d83174a1de62687430e0a0abc9542e96dfb Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 18:00:42 -0500 Subject: [PATCH 16/26] Fix imports --- .../org/elasticsearch/index/engine/SoftDeletesPolicy.java | 1 - .../java/org/elasticsearch/index/seqno/RetentionLease.java | 4 ---- .../org/elasticsearch/index/seqno/RetentionLeaseStats.java | 1 - 3 files changed, 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java b/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java index c4375a03da3ae..49b8f9d3483f2 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java +++ b/server/src/main/java/org/elasticsearch/index/engine/SoftDeletesPolicy.java @@ -29,7 +29,6 @@ import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.translog.Translog; -import java.util.Collection; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.LongSupplier; diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java index 258c9df698315..09edd1065c136 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java @@ -25,12 +25,8 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collection; import java.util.Locale; -import java.util.Map; import java.util.Objects; -import java.util.function.Function; -import java.util.stream.Collectors; /** * A "shard history retention lease" (or "retention lease" for short) is conceptually a marker containing a retaining sequence number such diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java index 8919b0c41310c..dfeb793979f8f 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java @@ -26,7 +26,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; -import java.util.Collection; import java.util.Objects; /** From 80b08b395adf8952985bcf82ea65c1e9eb0a2528 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 18:16:57 -0500 Subject: [PATCH 17/26] Fix some naming --- .../index/seqno/ReplicationTracker.java | 6 +++--- .../elasticsearch/index/seqno/RetentionLeases.java | 8 ++++---- .../index/engine/InternalEngineTests.java | 8 ++++---- .../seqno/ReplicationTrackerRetentionLeaseTests.java | 12 ++++++------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java index 1af1033f5dcfa..34ec443a5404a 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java @@ -190,16 +190,16 @@ public RetentionLeases getRetentionLeases() { // the primary calculates the non-expired retention leases and syncs them to replicas final long currentTimeMillis = currentTimeMillisSupplier.getAsLong(); final long retentionLeaseMillis = indexSettings.getRetentionLeaseMillis(); - final Map> partition = retentionLeases + final Map> partitionByExpiration = retentionLeases .leases() .stream() .collect(Collectors.groupingBy(lease -> currentTimeMillis - lease.timestamp() > retentionLeaseMillis)); - if (partition.get(true) == null) { + if (partitionByExpiration.get(true) == null) { // early out as no retention leases have expired return retentionLeases; } final Collection nonExpiredLeases = - partition.get(false) != null ? partition.get(false) : Collections.emptyList(); + partitionByExpiration.get(false) != null ? partitionByExpiration.get(false) : Collections.emptyList(); retentionLeases = new RetentionLeases(operationPrimaryTerm, retentionLeases.version() + 1, nonExpiredLeases); } /* diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index 01ce3ec0ff03f..d1057f8f3cf03 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -191,19 +191,19 @@ public static RetentionLeases decodeRetentionLeases(final String encodedRetentio final long primaryTerm = Long.parseLong(encodedRetentionLeases.substring("primary_term:".length(), firstSemicolon)); final int secondSemicolon = encodedRetentionLeases.indexOf(";", firstSemicolon + 1); final long version = Long.parseLong(encodedRetentionLeases.substring(firstSemicolon + 1 + "version:".length(), secondSemicolon)); - final Collection retentionLeases; + final Collection leases; if (secondSemicolon + 1 == encodedRetentionLeases.length()) { - retentionLeases = Collections.emptyList(); + leases = Collections.emptyList(); } else { assert Arrays.stream(encodedRetentionLeases.substring(secondSemicolon + 1).split(",")) .allMatch(s -> s.matches("id:[^:;,]+;retaining_seq_no:\\d+;timestamp:\\d+;source:[^:;,]+")) : encodedRetentionLeases; - retentionLeases = Arrays.stream(encodedRetentionLeases.substring(secondSemicolon + 1).split(",")) + leases = Arrays.stream(encodedRetentionLeases.substring(secondSemicolon + 1).split(",")) .map(RetentionLease::decodeRetentionLease) .collect(Collectors.toList()); } - return new RetentionLeases(primaryTerm, version, retentionLeases); + return new RetentionLeases(primaryTerm, version, leases); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 017a865d74999..2af09be8831e0 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -5306,14 +5306,14 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); final long primaryTerm = randomLongBetween(1, Long.MAX_VALUE); final AtomicLong retentionLeasesVersion = new AtomicLong(); - final AtomicReference leasesHolder = new AtomicReference<>(RetentionLeases.EMPTY); + final AtomicReference retentionLeasesHolder = new AtomicReference<>(RetentionLeases.EMPTY); final List operations = generateSingleDocHistory(true, randomFrom(VersionType.INTERNAL, VersionType.EXTERNAL), 2, 10, 300, "2"); Randomness.shuffle(operations); Set existingSeqNos = new HashSet<>(); store = createStore(); engine = createEngine( - config(indexSettings, store, createTempDir(), newMergePolicy(), null, null, globalCheckpoint::get, leasesHolder::get)); + config(indexSettings, store, createTempDir(), newMergePolicy(), null, null, globalCheckpoint::get, retentionLeasesHolder::get)); assertThat(engine.getMinRetainedSeqNo(), equalTo(0L)); long lastMinRetainedSeqNo = engine.getMinRetainedSeqNo(); for (Engine.Operation op : operations) { @@ -5338,7 +5338,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { final String source = randomAlphaOfLength(8); leases.add(new RetentionLease(id, retainingSequenceNumber, timestamp, source)); } - leasesHolder.set(new RetentionLeases(primaryTerm, retentionLeasesVersion.get(), leases)); + retentionLeasesHolder.set(new RetentionLeases(primaryTerm, retentionLeasesVersion.get(), leases)); } if (rarely()) { settings.put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING.getKey(), randomLongBetween(0, 10)); @@ -5352,7 +5352,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { engine.flush(true, true); assertThat(Long.parseLong(engine.getLastCommittedSegmentInfos().userData.get(Engine.MIN_RETAINED_SEQNO)), equalTo(engine.getMinRetainedSeqNo())); - final RetentionLeases leases = leasesHolder.get(); + final RetentionLeases leases = retentionLeasesHolder.get(); if (leases.leases().isEmpty()) { assertThat(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.RETENTION_LEASES), equalTo("")); } else { diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index 6c7c01e89db7e..6f029825cc82d 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -95,7 +95,7 @@ public void testAddOrRenewRetentionLease() { public void testAddRetentionLeaseCausesRetentionLeaseSync() { final AllocationId allocationId = AllocationId.newInitializing(); - final Map retentionLeases = new HashMap<>(); + final Map retainingSequenceNumbers = new HashMap<>(); final AtomicBoolean invoked = new AtomicBoolean(); final AtomicReference reference = new AtomicReference<>(); final ReplicationTracker replicationTracker = new ReplicationTracker( @@ -114,7 +114,7 @@ public void testAddRetentionLeaseCausesRetentionLeaseSync() { leases.leases() .stream() .collect(Collectors.toMap(RetentionLease::id, RetentionLease::retainingSequenceNumber)), - equalTo(retentionLeases)); + equalTo(retainingSequenceNumbers)); }); reference.set(replicationTracker); replicationTracker.updateFromMaster( @@ -128,7 +128,7 @@ public void testAddRetentionLeaseCausesRetentionLeaseSync() { for (int i = 0; i < length; i++) { final String id = randomAlphaOfLength(8); final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE); - retentionLeases.put(id, retainingSequenceNumber); + retainingSequenceNumbers.put(id, retainingSequenceNumber); replicationTracker.addRetentionLease(id, retainingSequenceNumber, "test", ActionListener.wrap(() -> {})); // assert that the new retention lease callback was invoked assertTrue(invoked.get()); @@ -336,16 +336,16 @@ public void testReplicaIgnoresOlderRetentionLeasesVersion() { long version = 0; for (int i = 0; i < length; i++) { final int innerLength = randomIntBetween(0, 8); - final Collection retentionLeases = new ArrayList<>(); + final Collection leases = new ArrayList<>(); for (int j = 0; j < innerLength; j++) { - retentionLeases.add( + leases.add( new RetentionLease(i + "-" + j, randomNonNegativeLong(), randomNonNegativeLong(), randomAlphaOfLength(8))); version++; } if (rarely()) { primaryTerm++; } - retentionLeasesCollection.add(new RetentionLeases(primaryTerm, version, retentionLeases)); + retentionLeasesCollection.add(new RetentionLeases(primaryTerm, version, leases)); } final Collection expectedLeases; if (length == 0 || retentionLeasesCollection.get(length - 1).leases().isEmpty()) { From ae0fc2aadb1d5705269dc4724b188b486e55a3e9 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 18:24:31 -0500 Subject: [PATCH 18/26] Fix checkstyle --- .../index/engine/InternalEngineTests.java | 11 +++++++++-- .../seqno/ReplicationTrackerRetentionLeaseTests.java | 1 - .../index/seqno/RetentionLeaseTests.java | 4 ---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 2af09be8831e0..1968307a8c765 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -5312,8 +5312,15 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { Randomness.shuffle(operations); Set existingSeqNos = new HashSet<>(); store = createStore(); - engine = createEngine( - config(indexSettings, store, createTempDir(), newMergePolicy(), null, null, globalCheckpoint::get, retentionLeasesHolder::get)); + engine = createEngine(config( + indexSettings, + store, + createTempDir(), + newMergePolicy(), + null, + null, + globalCheckpoint::get, + retentionLeasesHolder::get)); assertThat(engine.getMinRetainedSeqNo(), equalTo(0L)); long lastMinRetainedSeqNo = engine.getMinRetainedSeqNo(); for (Engine.Operation op : operations) { diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index 6f029825cc82d..9781d893a1d53 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -28,7 +28,6 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.IndexSettingsModule; -import java.lang.annotation.Retention; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java index 8ffbd791970ac..bd2dee78b05ed 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java @@ -24,12 +24,8 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasToString; From 08095cbfd09d4d52fdca9e25de408fae4d6443ae Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 19:12:49 -0500 Subject: [PATCH 19/26] Update server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java --- .../index/seqno/ReplicationTrackerRetentionLeaseTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index 9781d893a1d53..30fddb1421237 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -41,7 +41,6 @@ import java.util.stream.Collectors; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; From 8d9deae13c63abaf8dea1db6590bae002d21d83e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 19:54:08 -0500 Subject: [PATCH 20/26] Fix compilation --- .../index/seqno/ReplicationTrackerRetentionLeaseTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java index 30fddb1421237..9781d893a1d53 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java @@ -41,6 +41,7 @@ import java.util.stream.Collectors; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; From fce4da686f50f902c4809fac858b2c22a1606c16 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 19:56:13 -0500 Subject: [PATCH 21/26] Fix checkstyle --- .../java/org/elasticsearch/index/seqno/RetentionLeasesTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java index bc6b39c8d8814..33cc83f602860 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeasesTests.java @@ -25,7 +25,6 @@ import java.util.Collections; import java.util.List; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; From 76bb46b058061ef4cb2b9cc501c6934e888d0e02 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 19:58:35 -0500 Subject: [PATCH 22/26] Inline --- .../elasticsearch/index/seqno/RetentionLeaseStats.java | 1 + .../org/elasticsearch/index/seqno/RetentionLeases.java | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java index dfeb793979f8f..14f485d314928 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseStats.java @@ -88,6 +88,7 @@ public void writeTo(final StreamOutput out) throws IOException { public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { builder.startObject("retention_leases"); { + builder.field("primary_term", retentionLeases.primaryTerm()); builder.field("version", retentionLeases.version()); builder.startArray("leases"); { diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java index d1057f8f3cf03..5a9d9e333b27b 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLeases.java @@ -70,7 +70,7 @@ public long version() { * @return true if this retention leases collection supercedes the specified retention lease collection, otherwise false */ public boolean supersedes(final RetentionLeases that) { - return primaryTerm() > that.primaryTerm() || primaryTerm() == that.primaryTerm() && version() > that.version(); + return primaryTerm > that.primaryTerm || primaryTerm == that.primaryTerm && version > that.version; } private final Map leases; @@ -170,9 +170,9 @@ public static String encodeRetentionLeases(final RetentionLeases retentionLeases return String.format( Locale.ROOT, "primary_term:%d;version:%d;%s", - retentionLeases.primaryTerm(), - retentionLeases.version(), - retentionLeases.leases().stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(","))); + retentionLeases.primaryTerm, + retentionLeases.version, + retentionLeases.leases.values().stream().map(RetentionLease::encodeRetentionLease).collect(Collectors.joining(","))); } /** From 44c5e0f7df9be246fce7c5f03ed765712cde5718 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 19:59:17 -0500 Subject: [PATCH 23/26] Inline --- .../org/elasticsearch/index/seqno/RetentionLease.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java index 09edd1065c136..e1d362d98764a 100644 --- a/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java +++ b/server/src/main/java/org/elasticsearch/index/seqno/RetentionLease.java @@ -157,10 +157,10 @@ static String encodeRetentionLease(final RetentionLease retentionLease) { return String.format( Locale.ROOT, "id:%s;retaining_seq_no:%d;timestamp:%d;source:%s", - retentionLease.id(), - retentionLease.retainingSequenceNumber(), - retentionLease.timestamp(), - retentionLease.source()); + retentionLease.id, + retentionLease.retainingSequenceNumber, + retentionLease.timestamp, + retentionLease.source); } /** From bc96632e1782caf7671486b0e63a29d35b384e91 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 21:35:11 -0500 Subject: [PATCH 24/26] Fix test --- .../org/elasticsearch/index/engine/InternalEngineTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 1968307a8c765..25b0c9e00cb7d 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -5361,7 +5361,9 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException { equalTo(engine.getMinRetainedSeqNo())); final RetentionLeases leases = retentionLeasesHolder.get(); if (leases.leases().isEmpty()) { - assertThat(engine.getLastCommittedSegmentInfos().getUserData().get(Engine.RETENTION_LEASES), equalTo("")); + assertThat( + engine.getLastCommittedSegmentInfos().getUserData().get(Engine.RETENTION_LEASES), + equalTo("primary_term:" + primaryTerm + ";version:" + retentionLeasesVersion.get() + ";")); } else { assertThat( engine.getLastCommittedSegmentInfos().getUserData().get(Engine.RETENTION_LEASES), From becb055cc666d968fd4fbc6384e0e2039fe84deb Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 31 Jan 2019 21:36:06 -0500 Subject: [PATCH 25/26] Adjust docs --- docs/reference/indices/flush.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/indices/flush.asciidoc b/docs/reference/indices/flush.asciidoc index fdfcd80ecd463..ea8667aa1b713 100644 --- a/docs/reference/indices/flush.asciidoc +++ b/docs/reference/indices/flush.asciidoc @@ -104,7 +104,7 @@ which returns something similar to: "sync_id" : "AVvFY-071siAOuFGEO9P", <1> "max_unsafe_auto_id_timestamp" : "-1", "min_retained_seq_no" : "0", - "retention_leases" : "id:replica-0;retaining_seq_no:0;timestamp:1547235588;source:replica" + "retention_leases" : "primary_term:1;version:1;id:replica-0;retaining_seq_no:0;timestamp:1547235588;source:replica" }, "num_docs" : 0 } @@ -119,7 +119,7 @@ which returns something similar to: // TESTRESPONSE[s/"translog_uuid" : "hnOG3xFcTDeoI_kvvvOdNA"/"translog_uuid": $body.indices.twitter.shards.0.0.commit.user_data.translog_uuid/] // TESTRESPONSE[s/"history_uuid" : "XP7KDJGiS1a2fHYiFL5TXQ"/"history_uuid": $body.indices.twitter.shards.0.0.commit.user_data.history_uuid/] // TESTRESPONSE[s/"sync_id" : "AVvFY-071siAOuFGEO9P"/"sync_id": $body.indices.twitter.shards.0.0.commit.user_data.sync_id/] -// TESTRESPONSE[s/"retention_leases" : "id:replica-0;retaining_seq_no:0;timestamp:1547235588;source:replica"/"retention_leases": $body.indices.twitter.shards.0.0.commit.user_data.retention_leases/] +// TESTRESPONSE[s/"retention_leases" : "primary_term:1;version:1;id:replica-0;retaining_seq_no:0;timestamp:1547235588;source:replica"/"retention_leases": $body.indices.twitter.shards.0.0.commit.user_data.retention_leases/] <1> the `sync id` marker [float] From 73fb8f51b735ce9763ee230bee554c66ba4b92f5 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 1 Feb 2019 09:34:05 -0500 Subject: [PATCH 26/26] Disable BWC --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index e5bc1ab3ba986..22505ed69a66d 100644 --- a/build.gradle +++ b/build.gradle @@ -159,8 +159,8 @@ task verifyVersions { * the enabled state of every bwc task. It should be set back to true * after the backport of the backcompat code is complete. */ -final boolean bwc_tests_enabled = true -final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ +final boolean bwc_tests_enabled = false +final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/37951" /* place a PR link here when committing bwc changes */ if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")