Skip to content
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.

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

Write translog opSize twice #7735

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@dakrone
Copy link
Member

dakrone commented Sep 16, 2014

Adds a new greedyRead method that will read the operation sizes, then
consume that many bytes in the stream, validating the checksum without
transforming the bytes into a Translog.Operation. If the bytes are not
corrupt they are read into an operation and returned.

This new greedyRead method is used when reading translogs from disk,
where they may have corrupted sizes that can cause OOMEs

Adds a new test with a translog with a corrupted opSize and removes the
@AwaitsFix from the testTranslogCorruption in
AbstractSimpleTranslogTests.

Write translog opSize twice
Adds a new `greedyRead` method that will read the operation sizes, then
consume that many bytes in the stream, validating the checksum without
transforming the bytes into a `Translog.Operation`. If the bytes are not
corrupt they are read into an operation and returned.

This new `greedyRead` method is used when reading translogs from disk,
where they may have corrupted sizes that can cause OOMEs

Adds a new test with a translog with a corrupted opSize and removes the
`@AwaitsFix` from the testTranslogCorruption in
`AbstractSimpleTranslogTests`.
public Translog.Operation greedyRead(StreamInput in) throws IOException {
int opSize = readAndVerifyOpSize(in);

byte[] bytes = new byte[opSize];

This comment has been minimized.

Copy link
@s1monw

s1monw Sep 16, 2014

Contributor

maybe we can use ByteUtils.readIntLE(bytes, opsize-4) here?

This comment has been minimized.

Copy link
@dakrone

dakrone Sep 16, 2014

Author Member

Oh cool, didn't know about that method, I'll do that.

This comment has been minimized.

Copy link
@dakrone

dakrone Sep 16, 2014

Author Member

Actually, looking at it now, StreamOutput writes in big endian, so I'll add a .readIntBE(bytes, opsize-4) method

@@ -89,6 +139,7 @@ public void write(StreamOutput outStream, Translog.Operation op) throws IOExcept
// want to do here.
BufferedChecksumStreamOutput out = new BufferedChecksumStreamOutput(outStream);
outStream.writeInt(size); // opSize is not checksummed
outStream.writeInt(size); // second opSize

This comment has been minimized.

Copy link
@s1monw

s1monw Sep 16, 2014

Contributor

I wonder if we should use a hash here instead if some corruption wipes all bytes to 0 or some other value? maybe MurmurHash3

This comment has been minimized.

Copy link
@dakrone

dakrone Sep 16, 2014

Author Member

Yes, I agree, a hash would be nicer, I'll add that.

@dakrone

This comment has been minimized.

Copy link
Member Author

dakrone commented Sep 16, 2014

Closing this in favor of a bigger change for 1.5 to come.

@dakrone dakrone closed this Sep 16, 2014

@clintongormley clintongormley changed the title Write translog opSize twice Internal: Write translog opSize twice Sep 26, 2014

@jpountz jpountz removed the review label Oct 21, 2014

@dakrone dakrone deleted the dakrone:translog-corrupted-sizes branch Jun 1, 2015

@clintongormley clintongormley changed the title Internal: Write translog opSize twice Write translog opSize twice Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.