Skip to content

Commit ba285a5

Browse files
authored
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.
1 parent 3865435 commit ba285a5

File tree

7 files changed

+10
-11
lines changed

7 files changed

+10
-11
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public RetentionLease(final String id, final long retainingSequenceNumber, final
101101
// retention lease IDs can not contain these characters because they are used in encoding retention leases
102102
throw new IllegalArgumentException("retention lease ID can not contain any of [:;,] but was [" + id + "]");
103103
}
104-
if (retainingSequenceNumber < SequenceNumbers.UNASSIGNED_SEQ_NO) {
104+
if (retainingSequenceNumber < 0) {
105105
throw new IllegalArgumentException("retention lease retaining sequence number [" + retainingSequenceNumber + "] out of range");
106106
}
107107
if (timestamp < 0) {

server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -5313,7 +5313,7 @@ public void testKeepMinRetainedSeqNoByMergePolicy() throws IOException {
53135313
final List<RetentionLease> leases = new ArrayList<>(length);
53145314
for (int i = 0; i < length; i++) {
53155315
final String id = randomAlphaOfLength(8);
5316-
final long retainingSequenceNumber = randomLongBetween(0L, Math.max(0L, globalCheckpoint.get()));
5316+
final long retainingSequenceNumber = randomLongBetween(0, Math.max(0, globalCheckpoint.get()));
53175317
final long timestamp = randomLongBetween(0L, Long.MAX_VALUE);
53185318
final String source = randomAlphaOfLength(8);
53195319
leases.add(new RetentionLease(id, retainingSequenceNumber, timestamp, source));

server/src/test/java/org/elasticsearch/index/engine/SoftDeletesPolicyTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public void testSoftDeletesRetentionLock() {
4949
AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED);
5050
final AtomicLong[] retainingSequenceNumbers = new AtomicLong[randomIntBetween(0, 8)];
5151
for (int i = 0; i < retainingSequenceNumbers.length; i++) {
52-
retainingSequenceNumbers[i] = new AtomicLong(SequenceNumbers.UNASSIGNED_SEQ_NO);
52+
retainingSequenceNumbers[i] = new AtomicLong();
5353
}
5454
final Supplier<Collection<RetentionLease>> retentionLeasesSupplier =
5555
() -> {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ private void runExpirationTest(final boolean primaryMode) {
156156
replicationTracker.activatePrimaryMode(SequenceNumbers.NO_OPS_PERFORMED);
157157
}
158158
final long[] retainingSequenceNumbers = new long[1];
159-
retainingSequenceNumbers[0] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
159+
retainingSequenceNumbers[0] = randomLongBetween(0, Long.MAX_VALUE);
160160
if (primaryMode) {
161161
replicationTracker.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {}));
162162
} else {

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception {
6767
final Map<String, RetentionLease> currentRetentionLeases = new HashMap<>();
6868
for (int i = 0; i < length; i++) {
6969
final String id = randomValueOtherThanMany(currentRetentionLeases.keySet()::contains, () -> randomAlphaOfLength(8));
70-
final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
70+
final long retainingSequenceNumber = randomLongBetween(0, Long.MAX_VALUE);
7171
final String source = randomAlphaOfLength(8);
7272
final CountDownLatch latch = new CountDownLatch(1);
7373
final ActionListener<ReplicationResponse> listener = ActionListener.wrap(r -> latch.countDown(), e -> fail(e.toString()));
@@ -119,7 +119,7 @@ public void testRetentionLeasesSyncOnExpiration() throws Exception {
119119
final int length = randomIntBetween(1, 8);
120120
for (int i = 0; i < length; i++) {
121121
final String id = randomAlphaOfLength(8);
122-
final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
122+
final long retainingSequenceNumber = randomLongBetween(0, Long.MAX_VALUE);
123123
final String source = randomAlphaOfLength(8);
124124
final CountDownLatch latch = new CountDownLatch(1);
125125
final ActionListener<ReplicationResponse> listener = ActionListener.wrap(r -> latch.countDown(), e -> fail(e.toString()));

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.Collection;
2929
import java.util.List;
3030

31-
import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO;
3231
import static org.hamcrest.Matchers.contains;
3332
import static org.hamcrest.Matchers.containsString;
3433
import static org.hamcrest.Matchers.empty;
@@ -53,7 +52,7 @@ public void testEmptyId() {
5352
}
5453

5554
public void testRetainingSequenceNumberOutOfRange() {
56-
final long retainingSequenceNumber = randomLongBetween(Long.MIN_VALUE, UNASSIGNED_SEQ_NO - 1);
55+
final long retainingSequenceNumber = randomLongBetween(Long.MIN_VALUE, -1);
5756
final IllegalArgumentException e = expectThrows(
5857
IllegalArgumentException.class,
5958
() -> new RetentionLease("id", retainingSequenceNumber, randomNonNegativeLong(), "source"));
@@ -66,7 +65,7 @@ public void testTimestampOutOfRange() {
6665
final long timestamp = randomLongBetween(Long.MIN_VALUE, -1);
6766
final IllegalArgumentException e = expectThrows(
6867
IllegalArgumentException.class,
69-
() -> new RetentionLease("id", randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE), timestamp, "source"));
68+
() -> new RetentionLease("id", randomNonNegativeLong(), timestamp, "source"));
7069
assertThat(e, hasToString(containsString("retention lease timestamp [" + timestamp + "] out of range")));
7170
}
7271

@@ -87,7 +86,7 @@ public void testEmptySource() {
8786

8887
public void testRetentionLeaseSerialization() throws IOException {
8988
final String id = randomAlphaOfLength(8);
90-
final long retainingSequenceNumber = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
89+
final long retainingSequenceNumber = randomLongBetween(0, Long.MAX_VALUE);
9190
final long timestamp = randomNonNegativeLong();
9291
final String source = randomAlphaOfLength(8);
9392
final RetentionLease retentionLease = new RetentionLease(id, retainingSequenceNumber, timestamp, source);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ private void runExpirationTest(final boolean primary) throws IOException {
113113
final IndexShard indexShard = newStartedShard(primary, settings, new InternalEngineFactory());
114114
try {
115115
final long[] retainingSequenceNumbers = new long[1];
116-
retainingSequenceNumbers[0] = randomLongBetween(SequenceNumbers.NO_OPS_PERFORMED, Long.MAX_VALUE);
116+
retainingSequenceNumbers[0] = randomLongBetween(0, Long.MAX_VALUE);
117117
if (primary) {
118118
indexShard.addRetentionLease("0", retainingSequenceNumbers[0], "test-0", ActionListener.wrap(() -> {}));
119119
} else {

0 commit comments

Comments
 (0)