Add a shard snapshot current status debug string #118198
Conversation
elasticsearchmachine
added
needs:triage
DiannaHohensee
changed the title
The SnapshotShutdownProgressTracker will iterate through the SnapshotShardsService's list of shard snapshots and log their current status. SnapshotShardsService will now update a new debug string field in IndexShardSnapshotStatus as a shard snapshot operation proceeds so that the current state can be logged.
DiannaHohensee
force-pushed
the
2024/12/06/ES-10261-snapshot-debug-status
branch
from
3c06926 to
4ed1d23
Compare
DiannaHohensee
added
Team:Distributed Coordination
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
| @@ -98,6 +98,7 @@ public enum AbortStatus { | |||
| private long processedSize; | |||
| private String failure; | |||
| private final SubscribableListener<AbortStatus> abortListeners = new SubscribableListener<>(); | |||
| private String debugStatusString; | |||
There was a problem hiding this comment.
Probably needs to be volatile so that toString() and friends all see the latest value. And in that case there's no need to make the update method synchronized.
| private String debugStatusString; | |
| private volatile String debugStatusString; |
There was a problem hiding this comment.
Ah, the toString(). Changed 👍 .
| * Updates the string explanation for what the snapshot is actively doing right now. | ||
| */ | ||
| public synchronized void updateDebugStatusString(String statusString) { | ||
| this.debugStatusString = statusString; |
There was a problem hiding this comment.
Can we at least assert that statusString is not null (or empty)?
There was a problem hiding this comment.
Sure, added 👍
| @@ -461,6 +482,8 @@ public String toString() { | |||
| + processedSize | |||
| + ", failure='" | |||
| + failure | |||
| + "', debugStatusString='" | |||
There was a problem hiding this comment.
Not sure about exposing this to users as debugStatusString, which I believe might happen since this is logged at INFO sometimes. Seems a little overly technical. How about status or statusDescription?
There was a problem hiding this comment.
Agreed. updated to statusDescription 👍
| @@ -461,6 +482,8 @@ public String toString() { | |||
| + processedSize | |||
| + ", failure='" | |||
| + failure | |||
| + "', debugStatusString='" | |||
There was a problem hiding this comment.
Agreed. updated to statusDescription 👍
| * Updates the string explanation for what the snapshot is actively doing right now. | ||
| */ | ||
| public synchronized void updateDebugStatusString(String statusString) { | ||
| this.debugStatusString = statusString; |
There was a problem hiding this comment.
Sure, added 👍
| @@ -98,6 +98,7 @@ public enum AbortStatus { | |||
| private long processedSize; | |||
| private String failure; | |||
| private final SubscribableListener<AbortStatus> abortListeners = new SubscribableListener<>(); | |||
| private String debugStatusString; | |||
There was a problem hiding this comment.
Ah, the toString(). Changed 👍 .
| assert statusString != null; | ||
| assert statusString.isEmpty() == false; |
There was a problem hiding this comment.
Nit: should really check the value passed into the constructor too in order to be certain that the statusDescription field is never null or empty.
Add a shard snapshot current status debug string
SnapshotShardsService will update the new debug string field in
IndexShardSnapshotStatus as a shard snapshot operation proceeds so
that the current state can be logged. The
SnapshotShutdownProgressTracker will iterate through the
SnapshotShardsService's list of shard snapshots and log their current
status.
We want to know where in the code a shard snapshot operation
potentially gets stuck. This new field should be updated as
frequently as is reasonable to inform on the shard snapshot's progress.
Closes ES-10261