Skip to content

Checksum docvalues files when load store metadata #30851

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Checksum docvalues files when load store metadata #30851

wants to merge 1 commit into from

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented May 24, 2018

In CorruptedFileIT, we corrupt a random segment file then check that the corrupting store is marked as corrupted. However, if a DocValues file is flipped, we will trip an assertion in a FixedBitSet when opening a SoftDeletesDirectoryReaderWrapper. This commit makes sure that we verify the DocValues files when loading the metadata from a store.

Another option is to limit the set of corrupting files in the tests.

@dnhatn
Checksum docvalues files when load store metadata
79859f0 
In CorruptedFileIT, we corrupt a random segment file then check that the
corrupting store is marked as corrupted. However, if a DocValues file is
flipped, we will trip an assertion in a FixedBitSet when opening a
SoftDeletesDirectoryReaderWrapper. This commit makes sure that we verify
the DocValues files when loading the metadata from a store.

Another option is to limit the set of corrupting files in the tests.
@dnhatn dnhatn added >feature :Distributed Indexing/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. labels May 24, 2018
@dnhatn dnhatn requested review from bleskes and s1monw May 24, 2018 21:22
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 24, 2018

Pinging @elastic/es-distributed

@dnhatn
Copy link
Member Author

dnhatn commented May 24, 2018

The stack trace:

WARNING: Uncaught exception in thread: Thread[elasticsearch[node_s1][fetch_shard_store][T#4],5,TGRP-CorruptedFileIT]
java.lang.AssertionError: index=373, numBits=283
	at __randomizedtesting.SeedInfo.seed([CFF5E7C9656807DF]:0)
	at org.apache.lucene.util.FixedBitSet.get(FixedBitSet.java:181)
	at org.apache.lucene.index.PendingSoftDeletes.applySoftDeletes(PendingSoftDeletes.java:116)
	at org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper.wrap(SoftDeletesDirectoryReaderWrapper.java:115)
	at org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper$SoftDeletesSubReaderWrapper.wrap(SoftDeletesDirectoryReaderWrapper.java:95)
	at org.apache.lucene.index.FilterDirectoryReader$SubReaderWrapper.wrap(FilterDirectoryReader.java:56)
	at org.apache.lucene.index.FilterDirectoryReader$SubReaderWrapper.access$000(FilterDirectoryReader.java:51)
	at org.apache.lucene.index.FilterDirectoryReader.<init>(FilterDirectoryReader.java:83)
	at org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper.<init>(SoftDeletesDirectoryReaderWrapper.java:53)
	at org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper.<init>(SoftDeletesDirectoryReaderWrapper.java:49)
	at org.elasticsearch.common.lucene.Lucene.getExactNumDocs(Lucene.java:159)
	at org.elasticsearch.index.store.Store$MetadataSnapshot.loadMetadata(Store.java:887)
	at org.elasticsearch.index.store.Store$MetadataSnapshot.<init>(Store.java:797)
	at org.elasticsearch.index.store.Store.getMetadata(Store.java:293)
	at org.elasticsearch.index.store.Store.getMetadata(Store.java:258)

@s1monw
Copy link
Contributor

s1monw commented May 25, 2018

I don't think we can force the checksum on the metadata retrieval this is just too costly. We should try to find a better way of doing this.

@bleskes
Copy link
Contributor

bleskes commented May 28, 2018 •
edited
Loading

I wonder why this is an assertion and not a hard IndexOutOfBounds exception. Also, it feels like we should pre check this higher up and throw an Index corrupted exception if things don't add up?

@s1monw
Copy link
Contributor

s1monw commented May 29, 2018

@bleskes we try to make this not needed on the lucene end. stay tuned

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jun 4, 2018
@dnhatn
Upgrade to Lucene-7.4.0-snapshot-0a7c3f462f
c0feddb 
This snapshot includes:
- LUCENE-8341: Record soft deletes in SegmentCommitInfo
- LUCENE-8335: Enforce soft-deletes field up-front

LUCENE-8341 is required to resolve elastic#30851.
@dnhatn dnhatn mentioned this pull request Jun 4, 2018
Merged
@dnhatn dnhatn closed this in #31073 Jun 4, 2018
dnhatn added a commit that referenced this pull request Jun 4, 2018
@dnhatn
Upgrade to Lucene-7.4.0-snapshot-0a7c3f462f (#31073)
abe6115 
This snapshot includes:
- LUCENE-8341: Record soft deletes in SegmentCommitInfo which will resolve #30851
- LUCENE-8335: Enforce soft-deletes field up-front
dnhatn added a commit that referenced this pull request Jun 4, 2018
@dnhatn
Upgrade to Lucene-7.4.0-snapshot-0a7c3f462f (#31073)
c64181b 
This snapshot includes:
- LUCENE-8341: Record soft deletes in SegmentCommitInfo which will resolve #30851
- LUCENE-8335: Enforce soft-deletes field up-front
@dnhatn dnhatn deleted the checksum-dvd branch June 4, 2018 23:49
@dnhatn
Copy link
Member Author

dnhatn commented Jun 4, 2018

Thanks @s1monw and @bleskes for taking a look. This is resolved by LUCENE-8341.

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Jun 5, 2018
@dnhatn
Upgrade to Lucene-7.4.0-snapshot-0a7c3f462f (elastic#31073)
206fb22 
This snapshot includes:
- LUCENE-8341: Record soft deletes in SegmentCommitInfo which will resolve elastic#30851
- LUCENE-8335: Enforce soft-deletes field up-front
dnhatn added a commit that referenced this pull request Jun 5, 2018
@dnhatn
Upgrade to Lucene-7.4.0-snapshot-0a7c3f462f (#31073)
91de0ba 
This snapshot includes:
- LUCENE-8341: Record soft deletes in SegmentCommitInfo which will resolve #30851
- LUCENE-8335: Enforce soft-deletes field up-front
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s1monw s1monw Awaiting requested review from s1monw

@bleskes bleskes Awaiting requested review from bleskes

Assignees

No one assigned

Labels

:Distributed Indexing/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. >feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dnhatn @elasticmachine @s1monw @bleskes