Verify checksums on merge #7360
Conversation
| [[index-checksum-on-merge]]`index.checksum_on_merge`:: | ||
|
|
||
| Should merging first validate checksums on segments being merged? | ||
| Defaults to `false`. This is a dynamic setting. coming[1.4.0] |
There was a problem hiding this comment.
Should this be "Added [1.4.0]"? No one will see this in official docs until then right?
There was a problem hiding this comment.
afaik coming is automatically changed to added during the release process.
There was a problem hiding this comment.
yeah at this point we build our docs from the 1.x branch so your chance as immediate effect. It will be moved to added once we release it.
|
This looks good. Just a minor comment on the docs. |
| @@ -113,6 +113,7 @@ public void setUp() throws Exception { | |||
| super.setUp(); | |||
| defaultSettings = ImmutableSettings.builder() | |||
| .put(InternalEngine.INDEX_COMPOUND_ON_FLUSH, getRandom().nextBoolean()) | |||
| .put(InternalEngine.INDEX_CHECKSUM_ON_MERGE, getRandom().nextBoolean()) | |||
There was a problem hiding this comment.
I think we need a test that also checks that the live change works. Since you missed to register the setting in
org.elasticsearch.index.settings.IndexDynamicSettingsModule such a test should fail.
There was a problem hiding this comment.
Yes, I really wanted this too. I was beaten back by the fact that tests for any of the other IndexWriter options are integration tests (indexing stuff and looking at number of segments etc), instead of simply being unit tests that check that IWC was changed.
|
I updated this with a test extending simon's new base class, changing default to true. |
|
LGTM |
|
can you change the name of the issue to reflect the fact that we enable this feature rather than adding an option. Other than that LGTM thanks rob |
Enable lucene verification of checksums on segments before merging them. This prevents corruption from existing segments from silently slipping into newer merged segments. Closes #7360
Enable lucene verification of checksums on segments before merging them. This prevents corruption from existing segments from silently slipping into newer merged segments. Closes #7360
clintongormley
changed the title
clintongormley
changed the title
This just exposes the current lucene option
checkIntegrityAtMergefrom LiveIndexWriterConfig. When enabled, all parts of the index are verified before merging.