Skip to content

Fixes assertion check in RollupShardIndexer#68878

Merged
talevy merged 1 commit intoelastic:masterfrom
talevy:fixassertrollup
Feb 11, 2021
Merged

Fixes assertion check in RollupShardIndexer#68878
talevy merged 1 commit intoelastic:masterfrom
talevy:fixassertrollup

Conversation

@talevy
Copy link
Copy Markdown
Contributor

@talevy talevy commented Feb 11, 2021

This commit removes the assertion in RollupShardIndexer that verifies that
temporary files are deleted. Since it is the responsibility of the indexer
to instruct the OS to delete files, it may not do so in a timely manner. This
results in a potentially flaky assertion. Instead, a new unit test is introduced
that will introspect the indexer and assert that it had successfully called
for the files to be deleted.

Closes #68609.

This commit removes the assertion in RollupShardIndexer that verifies that
temporary files are deleted. Since it is the responsibility of the indexer
to instruct the OS to delete files, it may not do so in a timely manner. This
results in a potentially flaky assertion. Instead, a new unit test is introduced
that will introspect the indexer and assert that it had successfully called
for the files to be deleted.

Closes elastic#68609.
@talevy talevy added >test Issues or PRs that are addressing/adding tests :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Feb 11, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@talevy talevy requested a review from csoulios February 11, 2021 01:23
Copy link
Copy Markdown
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

LGTM!

@talevy talevy merged commit a0d8a89 into elastic:master Feb 11, 2021
@talevy talevy deleted the fixassertrollup branch February 11, 2021 23:07
talevy added a commit to talevy/elasticsearch that referenced this pull request Feb 16, 2021
This commit removes the assertion in RollupShardIndexer that verifies that
temporary files are deleted. Since it is the responsibility of the indexer
to instruct the OS to delete files, it may not do so in a timely manner. This
results in a potentially flaky assertion. Instead, a new unit test is introduced
that will introspect the indexer and assert that it had successfully called
for the files to be deleted.

Closes elastic#68609.
talevy added a commit that referenced this pull request Feb 16, 2021
This commit removes the assertion in RollupShardIndexer that verifies that
temporary files are deleted. Since it is the responsibility of the indexer
to instruct the OS to delete files, it may not do so in a timely manner. This
results in a potentially flaky assertion. Instead, a new unit test is introduced
that will introspect the indexer and assert that it had successfully called
for the files to be deleted.

backport of #68878.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v7.12.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] RollupActionIT testRollupIndexAndSetNewRollupPolicy failing

4 participants