Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only iterate the files that we recovered from the commit #9761

Merged
merged 1 commit into from Feb 19, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Feb 19, 2015

Today we use Directory#listAll() to find all the files we recovered. Yet,
this is not accurate since there might be leftovers etc. It's better to
only iterate over the known files from the segments info that we recovered.

@kimchy
Copy link
Member

kimchy commented Feb 19, 2015

LGTM

@bleskes
Copy link
Contributor

bleskes commented Feb 19, 2015

LGTM do we want a test that injects evil files?

@s1monw
Copy link
Contributor Author

s1monw commented Feb 19, 2015

LGTM do we want a test that injects evil files?

that one is going be available at the lucene release next to you soon

Today we use Directory#listAll() to find all the files we recovered. Yet,
this is not accurate since there might be leftovers etc. It's better to
only iterate over the known files from the segments info that we recovered.
@s1monw s1monw merged commit 50a56b6 into elastic:master Feb 19, 2015
@s1monw s1monw deleted the take_files_from_commit branch February 19, 2015 16:38
*/
public static Iterable<String> files(SegmentInfos infos) throws IOException {
final List<Collection<String>> list = new ArrayList<>();
for (SegmentCommitInfo info : infos) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this include the segments file? (e.g. call infos.files() instead) The old listAll() would have included it too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pushed a fix for this here e8f276b

s1monw added a commit to s1monw/elasticsearch that referenced this pull request Feb 20, 2015
s1monw added a commit to s1monw/elasticsearch that referenced this pull request Feb 20, 2015
@clintongormley clintongormley added :Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. and removed review labels Mar 19, 2015
@clintongormley clintongormley changed the title [RECOVERY] Only iterate the files that we recovered from the commit Only iterate the files that we recovered from the commit Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. >enhancement v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants