Skip to content

Report shard counts for ongoing snapshots#77622

Merged
arteam merged 9 commits intoelastic:masterfrom
arteam:report-in-progress-snapshots-stats
Sep 27, 2021
Merged

Report shard counts for ongoing snapshots#77622
arteam merged 9 commits intoelastic:masterfrom
arteam:report-in-progress-snapshots-stats

Conversation

@arteam
Copy link
Copy Markdown
Contributor

@arteam arteam commented Sep 13, 2021

For in-progress snapshots we can try to calculate the amount of successful shards based on the shard state. We can use a similar approach for failures.

Reference #76704

@arteam arteam changed the title DRAFT: Report shard counts for ongoing snapshots [DRAFT] Report shard counts for ongoing snapshots Sep 13, 2021
@arteam arteam changed the title [DRAFT] Report shard counts for ongoing snapshots Report shard counts for ongoing snapshots Sep 15, 2021
@arteam arteam force-pushed the report-in-progress-snapshots-stats branch 4 times, most recently from 09bed75 to c4d62e4 Compare September 16, 2021 15:11
@arteam arteam added :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v7.16.0 labels Sep 16, 2021
@arteam arteam marked this pull request as ready for review September 16, 2021 17:54
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Sep 16, 2021
@arteam arteam marked this pull request as draft September 16, 2021 18:23
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Sep 17, 2021

@elasticmachine update branch

@elastic elastic deleted a comment from elasticmachine Sep 17, 2021
@arteam arteam force-pushed the report-in-progress-snapshots-stats branch from cfe0c36 to 5027c16 Compare September 17, 2021 19:00
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Sep 20, 2021

@elasticmachine run elasticsearch-ci/part-1

@arteam arteam force-pushed the report-in-progress-snapshots-stats branch 2 times, most recently from ad2d7f7 to 0c6faaf Compare September 20, 2021 11:54
@arteam arteam force-pushed the report-in-progress-snapshots-stats branch from 0c6faaf to e476e42 Compare September 20, 2021 11:59
@arteam arteam marked this pull request as ready for review September 20, 2021 15:05
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Otherwise, the stable sorting isn't working because we get a different amount
of the successfully migrated shards.
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Sep 22, 2021

Hi @tlrx and @original-brownbear, could you guys please take a look at the PR, thanks!

@arteam arteam requested a review from DaveCTurner September 23, 2021 10:11

createIndexWithContent("test-idx-1");
String indexName = "test-idx-1";
createIndexWithContent(indexName, indexSettingsNoReplicas(4).build());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of forcing no replicas maybe we let the index settings be randomized as usual, and use getNumShards() to retrieve the expected number of shards to compare with later in the test?

@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Sep 27, 2021

@elasticmachine update branch

@arteam arteam requested a review from tlrx September 27, 2021 11:00
Copy link
Copy Markdown
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear 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, just a few minor points tests (lets not use actionGet) and the looping.

0,
Collections.emptyList(),
entry.shards().size(),
Math.toIntExact(entry.shards().stream().filter(c -> c.getValue().state() == SnapshotsInProgress.ShardState.SUCCESS).count()),
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.

Might be nice to calculate both this count and the failures list in a single loop by using a static method + Tuple or so? Afaik this may run on a transport thread so it's worth saving the cycles, especially as we scale to larger and larger clusters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I believe I can do that with a teeing collector.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, we can't, because we were calling another constructor and couldn't declare variables. I went ahead with removing of the constructor call and using a simple loop for calculating both fields.

@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Sep 27, 2021

@elasticmachine run elasticsearch-ci/part-1

@arteam arteam merged commit 52e73fe into elastic:master Sep 27, 2021
@arteam arteam deleted the report-in-progress-snapshots-stats branch September 27, 2021 17:24
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Sep 27, 2021

Thanks Tanguy and Armin!

@arteam arteam added the auto-backport Automatically create backport pull requests when merged label Sep 27, 2021
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 77622

arteam added a commit to arteam/elasticsearch that referenced this pull request Sep 27, 2021
For in-progress snapshots we can try to calculate the amount of successful shards based on the shard state. We can use a similar approach for failures.

Closes elastic#76704
arteam added a commit that referenced this pull request Sep 29, 2021
For in-progress snapshots we can try to calculate the amount of successful shards based on the shard state. We can use a similar approach for failures.

Closes #76704
arteam added a commit that referenced this pull request Sep 29, 2021
arteam added a commit that referenced this pull request Sep 29, 2021
arteam added a commit that referenced this pull request Sep 29, 2021
arteam added a commit that referenced this pull request Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Distributed Meta label for distributed team. v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants