Allow missing shard stats for restarted nodes for _snapshot/_status

[JeremyDahlgren]

Returns an empty shard stats for shard entries where stats were unavailable in the case where a node has been restarted or left the cluster. The change adds a description field to the SnapshotIndexShardStatus class that is used to include a message indicating why the stats are empty. To reduce confusion when stats are empty the stats values will all be -1 except for the time values which will be zero. This is in addition to the new description field.

This change was motivated by a desire to reduce latency for getting the stats for currently running snapshots. The stats can still be loaded later from the repository via a _snapshot/<repository>/snapshot/_status call when the snapshot has completed.

An API docs update is in elasticsearch-specification PR #4410.

Closes ES-10982

[elasticsearchmachine]

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

[elasticsearchmachine]

Hi @JeremyDahlgren, I've created a changelog YAML for you.

[DiannaHohensee]

Looks good. My only remaining concern is the to/fromXContent code. I never got that familiar with the logic. I think that it might be OK as is, since it's just reading and writing values. But the testing appears to be in SnapshotStatsTests via the createTestInstance() implementation and extending AbstractXContentTestCase. I wonder what would happen if the random number generation in createTestInstance() were extended to -1? For reference I was looking at the original commit that added the XContent code

[JeremyDahlgren]

Looks good. My only remaining concern is the to/fromXContent code. I never got that familiar with the logic. I think that it might be OK as is, since it's just reading and writing values. But the testing appears to be in SnapshotStatsTests via the createTestInstance() implementation and extending AbstractXContentTestCase. I wonder what would happen if the random number generation in createTestInstance() were extended to -1? For reference I was looking at the original commit that added the XContent code

I added an explicit test case for XContent round trip serialization for the missing stats, which failed initially and I added a check for it in the deserialization. Thanks for raising this. I also added an explicit XContent serialization test for the missing stats static method in SnapshotIndexShardStatus. The random number generation in createTestInstance() seems ok with the zero lower bound for normal values, while we use the -1 values as a special case for "missing" stats.

[DiannaHohensee]

I added an explicit test case for XContent round trip serialization for the missing stats, which failed initially and I added a check for in the deserialization. Thanks for raising this. I also added an explicit XContent serialization test for the missing stats static method in SnapshotIndexShardStatus. The random number generation in createTestInstance() seems ok with the zero lower bound for normal values, while we use the -1 values as a special case for "missing" stats.

Clever test extension, I like it. Yeah, your solution makes more sense, to test the particular set of missingStats values, not randomize -1 values with a bunch of other numbers, which wouldn't happen.

My last question is about those zeros you found in the fromXContent method, wondering where they got set (or the absence of being set, really) that way.

[DiannaHohensee]

LGTM, without the pre-existing behavior fix in b1fc83c

[DiannaHohensee]

still lgtm 👍