Automatic verification of all files that are being snapshotted with Snapshot/Restore #7159
Conversation
| @@ -396,8 +392,10 @@ protected BlobStoreIndexShardSnapshots buildBlobStoreIndexShardSnapshots(Immutab | |||
| */ | |||
| public SnapshotContext(SnapshotId snapshotId, ShardId shardId, IndexShardSnapshotStatus snapshotStatus) { | |||
| super(snapshotId, shardId); | |||
| store = indicesService.indexServiceSafe(shardId.getIndex()).shardInjectorSafe(shardId.id()).getInstance(Store.class); | |||
| IndexService indexService = indicesService.indexServiceSafe(shardId.getIndex()); | |||
| store = indexService.shardInjectorSafe(shardId.id()).getInstance(Store.class); | |||
There was a problem hiding this comment.
can we cast to InternalIndexShard and call #store() on it? I think its cleaner....
|
it looks good to me, maybe @rmuir can have a look at that verifying index input? |
| * mechanism that is used in some repository plugins (S3 for example). However, the checksum is only calculated on | ||
| * the first read. All consecutive reads of the same data are not used to calculate the checksum. | ||
| */ | ||
| static class VerifyingIndexInput extends ChecksumIndexInput { |
There was a problem hiding this comment.
This guy is fairly tricky: I know there are explicit tests for this indexinput, but do they test that seeking/re-reading does not impact the checksum computation?
There was a problem hiding this comment.
@rmuir I added a test to verify that, but I should probably rename readIndexInputFully into something like readIndexInputFullyWithRandomeSeeks to reflect that.
There was a problem hiding this comment.
Yes, that test takes care of my concerns. Maybe we can just add a comment to readBytes that our casts to 'int' are safe because seek catches us up? I stared hard at this thing, I dont see any problems, but it might help someone else. Thanks for doing this, I think it may be useful in the future for other purposes too (e.g. fold logic into ChecksumIndexInput).
|
looks good here. sorry for the delay in review! |
Adds automatic verification of all files that are being snapshotted. Closes elastic#5593
imotov
force-pushed
the
issue-5593-snpashot-checksum-lucene
branch
from
9ac7519
to
45ca777
Compare
clintongormley
changed the title
clintongormley
added
the
:Distributed/Snapshot/Restore
clintongormley
changed the title
Adds automatic verification of all files that are being snapshotted. Closes #5593