Skip to content

Fix testCreateAndRestoreSearchableSnapshot#55147

Merged
DaveCTurner merged 8 commits intoelastic:masterfrom
DaveCTurner:2020-04-14-testCreateAndRestoreSearchableSnapshot-failures
Apr 14, 2020
Merged

Fix testCreateAndRestoreSearchableSnapshot#55147
DaveCTurner merged 8 commits intoelastic:masterfrom
DaveCTurner:2020-04-14-testCreateAndRestoreSearchableSnapshot-failures

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

Fixes a couple of related failures in SearchableSnapshotsIntegTests.

Firstly, we were not correctly accounting for the case where the cache was so
small that some/all files were read directly; fixed this by only asserting that
the cache is definitely used if the corresponding node has a cache that's large
enough to hold the whole index.

Secondly, we were not permitting shards to be completely empty, which might be
the case (rarely) if there were not many documents indexed and the distribution
of IDs was a bit unlucky; fixed this by asserting that we get stats for at
least one file for the whole index, rather than for each shard separately.

Closes #55126

Fixes a couple of related failures in SearchableSnapshotsIntegTests.

Firstly, we were not correctly accounting for the case where the cache was so
small that some/all files were read directly; fixed this by only asserting that
the cache is definitely used if the corresponding node has a cache that's large
enough to hold the whole index.

Secondly, we were not permitting shards to be completely empty, which might be
the case (rarely) if there were not many documents indexed and the distribution
of IDs was a bit unlucky; fixed this by asserting that we get stats for at
least one file for the whole index, rather than for each shard separately.

Closes elastic#55126
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.8.0 labels Apr 14, 2020
@DaveCTurner DaveCTurner requested a review from tlrx April 14, 2020 09:59
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

builder.field("index_uuid", getIndexId().getId());
builder.startObject("shard");
{
builder.field("id", shardRouting.shardId());
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Kinda unrelated change, but this was useful for debugging.

? new ByteSizeValue(randomIntBetween(0, 10), ByteSizeUnit.KB)
? randomBoolean()
? new ByteSizeValue(randomIntBetween(0, 10), ByteSizeUnit.KB)
: new ByteSizeValue(randomIntBetween(0, 1000), ByteSizeUnit.BYTES)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Prior to this change the smallest nonzero cache size was 1kB which was typically larger than many of the files seen in this test, so I thought this might improve coverage a bit.

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, thanks David

.getState()
.nodes()
.getDataNodes()
.values()) {
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.

😷

@DaveCTurner DaveCTurner merged commit 9ba3c16 into elastic:master Apr 14, 2020
DaveCTurner added a commit that referenced this pull request Apr 14, 2020
Fixes a couple of related failures in SearchableSnapshotsIntegTests.

Firstly, we were not correctly accounting for the case where the cache was so
small that some/all files were read directly; fixed this by only asserting that
the cache is definitely used if the corresponding node has a cache that's large
enough to hold the whole index.

Secondly, we were not permitting shards to be completely empty, which might be
the case (rarely) if there were not many documents indexed and the distribution
of IDs was a bit unlucky; fixed this by asserting that we get stats for at
least one file for the whole index, rather than for each shard separately.

Closes #55126
@DaveCTurner DaveCTurner deleted the 2020-04-14-testCreateAndRestoreSearchableSnapshot-failures branch July 23, 2022 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >test Issues or PRs that are addressing/adding tests v7.8.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] SearchableSnapshotsIntegTests.testCreateAndRestoreSearchableSnapshot failing

4 participants