Skip to content

[Test] Test that sequence numbers are not pruned with retention lease#143825

Merged
tlrx merged 5 commits intoelastic:mainfrom
tlrx:2026/03/09-prune-seq-no-retention-lease
Mar 10, 2026
Merged

[Test] Test that sequence numbers are not pruned with retention lease#143825
tlrx merged 5 commits intoelastic:mainfrom
tlrx:2026/03/09-prune-seq-no-retention-lease

Conversation

@tlrx
Copy link
Copy Markdown
Member

@tlrx tlrx commented Mar 9, 2026

This commit adds a test in SeqNoPruningIT to ensure that sequence numbers are NOT pruned for operations that are still retained by a lease.

Follow up #143690

Edit: the test was made with Cursor, adjusted by me.

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v9.4.0 labels Mar 9, 2026
Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I left a few suggestions.

);

// add a retention lease holding at a sequence number in the middle of the indexed range
final long retentionLeaseSeqNo = randomLongBetween(1, totalDocs - 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can fetch the local checkpoint/max seq no of the primary instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed b21739d, I like the idea of selecting a random seq no in the ones available, instead of always picking up the max seq no.

final var primary = internalCluster().getInstance(IndicesService.class, clusterState.nodes().get(primaryShardNodeId).getName())
.getShardOrNull(new ShardId(resolveIndex(indexName), 0));

final var addLeaseLatch = new CountDownLatch(1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we have a transport action for this, can't we use that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I pushed 5826717

}
}
}
assertThat("expected to verify at least one shard", checkedShards, equalTo(1));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can remove the lease, index some data and force merge again to ensure that we eventually remove the seq_nos?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I pushed de3f874

@tlrx tlrx marked this pull request as ready for review March 9, 2026 11:57
@tlrx tlrx requested review from burqen and romseygeek March 9, 2026 11:57
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team. label Mar 9, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tlrx tlrx merged commit 9c8f15e into elastic:main Mar 10, 2026
35 of 36 checks passed
@tlrx tlrx deleted the 2026/03/09-prune-seq-no-retention-lease branch March 10, 2026 08:31
@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Mar 10, 2026

Thanks Francisco!

szybia added a commit to szybia/elasticsearch that referenced this pull request Mar 10, 2026
…locations

* upstream/main: (126 commits)
  Update KnnIndexTester to use more settings from datasets (elastic#143869)
  fix: dynamic template vector array is overridden by automatic dense_vector mapping (elastic#143733)
  ES|QL: Don't reuse the same alias for _fork column (elastic#143909)
  Close and initialize clients after each node upgrade in logsdb rolling upgrade tests. (elastic#143823)
  ESQL: Added GroupedTopNOperator for LIMIT BY, compute only (elastic#143476)
  Handle views in ResolveIndexAction (elastic#143561)
  Improve reindex rethrottle API in stateless (elastic#143771)
  Use a copy of the SearchExecutionContext for each Percolator execution (elastic#142765)
  Log the stacktrace when we encounter a deprecation warning for `default_metric` (elastic#143929)
  ESQL: evaluate ReferenceAttributes to potentially FieldAttributes for full-text functions restriction (elastic#143893)
  Add ClusterStateSerializationStats Serializatation Tests (elastic#142703)
  Adds Coordination Diagnostics Tests (elastic#142709)
  Upgrade Elasticsearch to Apache Lucene 10.4 (elastic#141882)
  ESQL: Add configurable bracket-based multi-value support for CSV reader (elastic#143890)
  time series es819 binary dv use up to a 1mb block size (elastic#143049)
  Dynamically enable / disable plugins in correspondence to stateless mode. (elastic#142147)
  ES|QL: Implement first/last_over_time for tdigest (elastic#143832)
  Document CHANGE_POINT limitation (elastic#143877)
  Fix OperationsOnSeqNoDisabledIndicesIT (elastic#143892)
  [Test] Test that sequence numbers are not pruned with retention lease (elastic#143825)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed Meta label for distributed team. >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants