From ba285a56a72655d189ee11aeed8c9eb8d2043ff9 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 30 Jan 2019 05:25:17 -0500 Subject: [PATCH] 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. --- .../java/org/elasticsearch/index/seqno/RetentionLease.java | 2 +- .../elasticsearch/index/engine/InternalEngineTests.java | 2 +- .../elasticsearch/index/engine/SoftDeletesPolicyTests.java | 2 +- .../index/seqno/ReplicationTrackerRetentionLeaseTests.java | 2 +- .../elasticsearch/index/seqno/RetentionLeaseSyncIT.java | 4 ++-- .../org/elasticsearch/index/seqno/RetentionLeaseTests.java | 7 +++---- .../index/shard/IndexShardRetentionLeaseTests.java | 2 +- 7 files changed, 10 insertions(+), 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 index 13e3381b11553..24d144d810d9c 100644 --- 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 index f57bff72fc57c..d984d1702f257 100644 --- 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 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 index 310e83e9d2cef..e15372d687e55 100644 --- 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> 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..90eb162374469 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/ReplicationTrackerRetentionLeaseTests.java +++ b/server/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 index 7d6e5fa2dc5a6..a99d0caea8e6c 100644 --- 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 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 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 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 index 500393f2cfac2..1a8d159c18757 100644 --- 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 index cd7d2a2c12cb8..26e67fb6dd264 100644 --- 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 {