Skip to content

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented Jul 3, 2025

See title

@elasticsearchmachine elasticsearchmachine added v9.2.0 serverless-linked Added by automation, don't add manually labels Jul 3, 2025
@ywangd ywangd added >non-issue :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. labels Jul 11, 2025
@ywangd ywangd requested a review from pxsalehi July 11, 2025 05:57
@ywangd ywangd marked this pull request as ready for review July 11, 2025 05:58
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Jul 11, 2025
@elasticsearchmachine
Copy link
Collaborator

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

IndexShard indexShard = indexService.createShard(shardRouting, globalCheckpointSyncer, retentionLeaseSyncer);
indexShard.addShardFailureCallback(onShardFailure);
indexShard.startRecovery(
final CheckedRunnable<RuntimeException> recoveryRunnable = () -> indexShard.startRecovery(
Copy link
Member

Choose a reason for hiding this comment

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

Using RuntimeException for CheckedRunnable doesn't seem right. Maybe just inline the indexShard.startRecovery?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure I inlined it. It is still an RuntimeException underlyingly. But at least we don't see it. I was trying to minimize the format changes. But it is pretty minimal either way.

@ywangd ywangd requested a review from pxsalehi July 15, 2025 10:23
@ywangd
Copy link
Member Author

ywangd commented Jul 18, 2025

@pxsalehi I could still use your approval on this stateful side of PR. Thanks!

@ywangd ywangd added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jul 21, 2025
@elasticsearchmachine elasticsearchmachine merged commit 6dd4d67 into elastic:main Jul 21, 2025
33 checks passed
@ywangd ywangd deleted the es-4123-fix branch July 21, 2025 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. >non-issue serverless-linked Added by automation, don't add manually Team:Distributed Indexing Meta label for Distributed Indexing team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants