Skip to content

Cleanup Old index-N Blobs in Repository Cleanup#49862

Merged
original-brownbear merged 2 commits intoelastic:masterfrom
original-brownbear:enhance-repo-cleanup
Dec 6, 2019
Merged

Cleanup Old index-N Blobs in Repository Cleanup#49862
original-brownbear merged 2 commits intoelastic:masterfrom
original-brownbear:enhance-repo-cleanup

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

Repository cleanup didn't deal with outdated index-N at the repository root, this change adds
cleaning up old index-N found in the repository.

Repository cleanup didn't deal with old index-N, this change adds
cleaning up all old index-N found in the repository.
@original-brownbear original-brownbear added >non-issue :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.6.0 labels Dec 5, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

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 - left minor comment about log traces that I don't find very useful and fills the test execution logs

internalCluster().startNodes(Settings.EMPTY);

final String repoName = "test-repo";
logger.info("--> creating repository");
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.

I'm not sure such info logs are very helpful

Copy link
Copy Markdown
Contributor Author

@original-brownbear original-brownbear Dec 6, 2019

Choose a reason for hiding this comment

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

I gotta disagree here :) I find these super helpful when it comes to debugging test failures from uncaught exceptions (esp. failing asserts on some non-test thread) where without these you often have a very hard time figuring out where exactly you were in the test at the time of the failure.

I wonder if we could do a better job on the logging side though ... there's not much point actually logging this stuff to the output on passing tests (basically like we have on the CLI). We could bring that up with core/infra?

@original-brownbear
Copy link
Copy Markdown
Contributor Author

Thanks Tanguy! Let's look into a better approach to logging elsewhere IMO, hope you don't mind :)

@original-brownbear original-brownbear merged commit dbdea13 into elastic:master Dec 6, 2019
@original-brownbear original-brownbear deleted the enhance-repo-cleanup branch December 6, 2019 09:28
original-brownbear added a commit that referenced this pull request Dec 9, 2019
* Cleanup Old index-N Blobs in Repository Cleanup

Repository cleanup didn't deal with old index-N, this change adds
cleaning up all old index-N found in the repository.
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
* Cleanup Old index-N Blobs in Repository Cleanup

Repository cleanup didn't deal with old index-N, this change adds
cleaning up all old index-N found in the repository.

* add test
@mfussenegger mfussenegger mentioned this pull request Mar 24, 2020
37 tasks
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 >non-issue v7.6.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants