Skip to content

Conversation

@DiannaHohensee
Copy link
Contributor

@DiannaHohensee DiannaHohensee commented Feb 27, 2025

Improves the information in the IndexShardSnapshotStatus's
statusDescription field to include the success/failure of the remote
call to the master node to update the shard snapshot state. This
allows us to see if there is a discrepancy between the state of the
data node and the master node.

Closes ES-10991

@DiannaHohensee DiannaHohensee added >enhancement :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination Meta label for Distributed Coordination team labels Feb 27, 2025
@DiannaHohensee DiannaHohensee self-assigned this Feb 27, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@DiannaHohensee DiannaHohensee force-pushed the 2025/02/27/ES-10991-status-description branch from fdc0862 to f482392 Compare February 27, 2025 21:48
@elastic elastic deleted a comment from elasticsearchmachine Feb 27, 2025
localShard.getValue().getShardSnapshotResult(),
() -> null
(outcomeInfoString) -> localShard.getValue().updateStatusDescription(
Strings.format("""
Copy link
Contributor Author

@DiannaHohensee DiannaHohensee Feb 27, 2025

Choose a reason for hiding this comment

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

This could theoretically grow without bound -- appending the previous status string. I figure it's unlikely to happen, since we'd stop once a remote call successfully updates the cluster state. We'd have to keep rolling over the master, and keep failing to get majority, or persistence in the blob store for serverless.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah could we impose a limit here just in case? Simply truncating the string if it exceeds 1000 characters or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 👍 I used substring: a bit wordy, let me know if there's some other/better standard way of doing it.

elasticsearchmachine and others added 2 commits February 27, 2025 21:55
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM hopefully this helps

@DiannaHohensee DiannaHohensee merged commit ba5a9c8 into elastic:main Feb 28, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Distributed Coordination Meta label for Distributed Coordination team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants