Fix limit on retaining sequence number (#37992)

We only assign non-negative sequence numbers to operations, so the lower limit on retaining sequence numbers should be that it is non-negative only.
elastic · Jan 30, 2019 · ba285a5 · ba285a5
1 parent 3865435
commit ba285a5
Show file tree

Hide file tree

Showing 7 changed files with 10 additions and 11 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
@@ -101,7 +101,7 @@ public RetentionLease(final String id, final long retainingSequenceNumber, final
             // retention lease IDs can not contain these characters because they are used in encoding retention leases
             throw new IllegalArgumentException("retention lease ID can not contain any of [:;,] but was [" + id + "]");
         }
-        if (retainingSequenceNumber < SequenceNumbers.UNASSIGNED_SEQ_NO) {
+        if (retainingSequenceNumber < 0) {
             throw new IllegalArgumentException("retention lease retaining sequence number [" + retainingSequenceNumber + "] out of range");
         }
         if (timestamp < 0) {

diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java
@@ -5313,7 +5313,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException {
                 final List<RetentionLease> leases = new ArrayList<>(length);
                 for (int i = 0; i < length; i++) {
                     final String id = randomAlphaOfLength(8);
-                    final long retainingSequenceNumber = randomLongBetween(0L, Math.max(0L, globalCheckpoint.get()));
+                    final long retainingSequenceNumber = randomLongBetween(0, Math.max(0, globalCheckpoint.get()));
                     final long timestamp = randomLongBetween(0L, Long.MAX_VALUE);
                     final String source = randomAlphaOfLength(8);
                     leases.add(new RetentionLease(id, retainingSequenceNumber, timestamp, source));

diff --git a/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java b/server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java
@@ -49,7 +49,7 @@ public void testSoftDeletesRetentionLock() {
         AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED);
         final AtomicLong[] retainingSequenceNumbers = new AtomicLong[randomIntBetween(0, 8)];
         for (int i = 0; i < retainingSequenceNumbers.length; i++) {
-            retainingSequenceNumbers[i] = new AtomicLong(SequenceNumbers.UNASSIGNED_SEQ_NO);
+            retainingSequenceNumbers[i] = new AtomicLong();
         }
         final Supplier<Collection<RetentionLease>> retentionLeasesSupplier =
                 () -> {

diff --git a/...er/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java b/...er/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java
@@ -156,7 +156,7 @@ private void runExpirationTest(final boolean primaryMode) {
             replicationTracker.activatePrimaryMode(SequenceNumbers.NO_OPS_PERFORMED);
         }
         final long[] retainingSequenceNumbers = new long[1];
-        retainingSequenceNumbers[0] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
+        retainingSequenceNumbers[0] = randomLongBetween(0, Long.MAX_VALUE);
         if (primaryMode) {
             replicationTracker.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {}));
         } else {

diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncIT.java
@@ -67,7 +67,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception {
         final Map<String, RetentionLease> currentRetentionLeases = new HashMap<>();
         for (int i = 0; i < length; i++) {
             final String id = randomValueOtherThanMany(currentRetentionLeases.keySet()::contains, () -> randomAlphaOfLength(8));
-            final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
+            final long retainingSequenceNumber = randomLongBetween(0, Long.MAX_VALUE);
             final String source = randomAlphaOfLength(8);
             final CountDownLatch latch = new CountDownLatch(1);
             final ActionListener<ReplicationResponse> listener = ActionListener.wrap(r -> latch.countDown(), e -> fail(e.toString()));
@@ -119,7 +119,7 @@ public void testRetentionLeasesSyncOnExpiration() throws Exception {
         final int length = randomIntBetween(1, 8);
         for (int i = 0; i < length; i++) {
             final String id = randomAlphaOfLength(8);
-            final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
+            final long retainingSequenceNumber = randomLongBetween(0, Long.MAX_VALUE);
             final String source = randomAlphaOfLength(8);
             final CountDownLatch latch = new CountDownLatch(1);
             final ActionListener<ReplicationResponse> listener = ActionListener.wrap(r -> latch.countDown(), e -> fail(e.toString()));

diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseTests.java
@@ -28,7 +28,6 @@
 import java.util.Collection;
 import java.util.List;
 
-import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
@@ -53,7 +52,7 @@ public void testEmptyId() {
     }
 
     public void testRetainingSequenceNumberOutOfRange() {
-        final long retainingSequenceNumber = randomLongBetween(Long.MIN_VALUE, UNASSIGNED_SEQ_NO - 1);
+        final long retainingSequenceNumber = randomLongBetween(Long.MIN_VALUE, -1);
         final IllegalArgumentException e = expectThrows(
                 IllegalArgumentException.class,
                 () -> new RetentionLease("id", retainingSequenceNumber, randomNonNegativeLong(), "source"));
@@ -66,7 +65,7 @@ public void testTimestampOutOfRange() {
         final long timestamp = randomLongBetween(Long.MIN_VALUE, -1);
         final IllegalArgumentException e = expectThrows(
                 IllegalArgumentException.class,
-                () -> new RetentionLease("id", randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE), timestamp, "source"));
+                () -> new RetentionLease("id", randomNonNegativeLong(), timestamp, "source"));
         assertThat(e, hasToString(containsString("retention lease timestamp [" + timestamp + "] out of range")));
     }
 
@@ -87,7 +86,7 @@ public void testEmptySource() {
 
     public void testRetentionLeaseSerialization() throws IOException {
         final String id = randomAlphaOfLength(8);
-        final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
+        final long retainingSequenceNumber = randomLongBetween(0, Long.MAX_VALUE);
         final long timestamp = randomNonNegativeLong();
         final String source = randomAlphaOfLength(8);
         final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, timestamp, source);

diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardRetentionLeaseTests.java
@@ -113,7 +113,7 @@ private void runExpirationTest(final boolean primary) throws IOException {
         final IndexShard indexShard = newStartedShard(primary, settings, new InternalEngineFactory());
         try {
             final long[] retainingSequenceNumbers = new long[1];
-            retainingSequenceNumbers[0] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
+            retainingSequenceNumbers[0] = randomLongBetween(0, Long.MAX_VALUE);
             if (primary) {
                 indexShard.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {}));
             } else {