Add checksum to snapshot metadata files #12002
Conversation
imotov
added
>enhancement
v2.0.0-beta1
review
resiliency
:Distributed/Snapshot/Restore
| Streams.readFully(inputStream, buffer); | ||
| } | ||
| int location = randomIntBetween(0, buffer.length - 1); | ||
| buffer[location] = (byte)(buffer[location] ^ 42); |
There was a problem hiding this comment.
can we compute CRC on buffer before and after introducing this corrupted byte. We can assume() the checksum has actually changed, so the test does not fail rarely?
| * A version of {@link BlobStoreFormat} that can automatically resolve blob names base on snapshot id | ||
| * and writes blobs in atomic manner | ||
| */ | ||
| public abstract class AtomicSnapshotBlobStoreFormat<T> extends SnapshotBlobStoreFormat<T> { |
There was a problem hiding this comment.
can we just fold this into thr default and always make the move?
imotov
force-pushed
the
issue-11589-add-checksum-to-snapshot-files
branch
from
9b24fd0
to
395126e
Compare
| * @param name blob name | ||
| * @throws IOException | ||
| */ | ||
| public void write(T obj, BlobContainer blobContainer, String name) throws IOException { |
There was a problem hiding this comment.
I think we should use writeAtomic everywhere just to reduce the complexity.
There was a problem hiding this comment.
I tried to do that but it creates a lot of inefficiency on slow repositories and problems while trying to create a snapshot with the same name as previously failed snapshot. We can do this switch, but I would prefer move it into another PR.
| MetaData.Builder.toXContent(metaData, builder, snapshotOnlyFormatParams); | ||
| builder.endObject(); | ||
| builder.flush(); | ||
| private BlobStoreFormat<IndexMetaData> indexMetaDataFormat(Version version) { |
There was a problem hiding this comment.
And here, you removed all the Javadoc! :)
There was a problem hiding this comment.
It's a different method. So I didn't remove javadoc, I just didn't add it :)
|
Left some comments |
|
@dakrone thanks for the review. I pushed fixes. |
|
|
||
| protected final boolean compress; | ||
|
|
||
| public final String codec; |
|
Left a couple more comments, other than that I have a question, I see you creating and passing around a |
|
@dakrone I pushed the revision. Currently, |
|
LGTM |
This commit adds checksum to snapshot files that store global and index based metadata as well as shard information. Closes elastic#11589
imotov
force-pushed
the
issue-11589-add-checksum-to-snapshot-files
branch
from
c1626fc
to
bfbee38
Compare
This commit adds checksum to snapshot files that store global and index based metadata as well as shard information.
Closes #11589