Core: Don't verify adler32 for lucene 3.x terms dict/terms index. #9142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These files are not write-once, but the 0.20.x checksum code didn't handle this properly, so we can't use the adler32.
|
LGTM. |
|
I added a commit enabling testing of 0.20.6 index. It fails with the checksum error without this fix. |
|
Still looks good! |
|
I thought about this .CFS case all day. For now, I would like the additional added logic (see latest commit) to avoid false checksum failures. If we want to throw an error on old versions i would like to separate that from doing the right thing here. Also note again we don't need this cruft on master, just 1.x |
|
Obviously this one is not really easy to test, our backwards tests config currently does not work with such ancient versions. But I know these old segments are still floating around, because I see people still hitting https://issues.apache.org/jira/browse/LUCENE-5975 on 1.3.x versions and so on. Enjoy reviewing :) |
|
looks good though. can we maybe have a simple unittest in StoreTests that checks if your conditionals are working ie with some bogus StoreFileMetadata? |
|
Good idea. We might as well put that fake segmentinfowriter to use. |
|
I added tests for these cases with fake metadata. |
|
very cool thanks rob! LGTM |
rmuir
added a commit
that referenced
this pull request
Jan 6, 2015
Don't verify adler32 for lucene 3.x terms dictionary (.tis), terms index (.tii), 3.0-3.3 compound file (.cfs), 3.x segments_N, or 3.x segments.gen. These files are not write-once, but the 0.20.x checksum code didn't handle .tis/.tii properly, so we can't use the adler32. Even older versions will have the same problems with other cases. Closes #9142
rmuir
added a commit
that referenced
this pull request
Jan 6, 2015
Don't verify adler32 for lucene 3.x terms dictionary (.tis), terms index (.tii), 3.0-3.3 compound file (.cfs), 3.x segments_N, or 3.x segments.gen. These files are not write-once, but the 0.20.x checksum code didn't handle .tis/.tii properly, so we can't use the adler32. Even older versions will have the same problems with other cases. Closes #9142 Conflicts: src/main/java/org/elasticsearch/index/store/Store.java src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java
rmuir
added a commit
that referenced
this pull request
Jan 6, 2015
Don't verify adler32 for lucene 3.x terms dictionary (.tis), terms index (.tii), 3.0-3.3 compound file (.cfs), 3.x segments_N, or 3.x segments.gen. These files are not write-once, but the 0.20.x checksum code didn't handle .tis/.tii properly, so we can't use the adler32. Even older versions will have the same problems with other cases. Closes #9142 Conflicts: src/main/java/org/elasticsearch/index/store/Store.java src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java
clintongormley
changed the title
clintongormley
added
:Core/Infra/Core
mute
pushed a commit
to mute/elasticsearch
that referenced
this pull request
Jul 29, 2015
Don't verify adler32 for lucene 3.x terms dictionary (.tis), terms index (.tii), 3.0-3.3 compound file (.cfs), 3.x segments_N, or 3.x segments.gen. These files are not write-once, but the 0.20.x checksum code didn't handle .tis/.tii properly, so we can't use the adler32. Even older versions will have the same problems with other cases. Closes elastic#9142 Conflicts: src/main/java/org/elasticsearch/index/store/Store.java src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java
mute
pushed a commit
to mute/elasticsearch
that referenced
this pull request
Jul 29, 2015
Don't verify adler32 for lucene 3.x terms dictionary (.tis), terms index (.tii), 3.0-3.3 compound file (.cfs), 3.x segments_N, or 3.x segments.gen. These files are not write-once, but the 0.20.x checksum code didn't handle .tis/.tii properly, so we can't use the adler32. Even older versions will have the same problems with other cases. Closes elastic#9142 Conflicts: src/main/java/org/elasticsearch/index/store/Store.java src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These files are not write-once, but the 0.20.x checksum code
didn't handle this properly, so we can't use the adler32.