Commitlog replay may fail if last mutation is within 4 bytes of end of segment
Patch by Jeff Jirsa; Reviewed by Branimir Lambov for CASSANDRA-13282 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/beb9658d Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/beb9658d Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/beb9658d Branch: refs/heads/trunk Commit: beb9658dd5e18e3a6a4e8431b6549ae4c33365a9 Parents: 5ef8a8b Author: Jeff Jirsa <j...@jeffjirsa.net> Authored: Sun Mar 12 21:54:04 2017 -0700 Committer: Jeff Jirsa <j...@jeffjirsa.net> Committed: Sun Mar 12 21:54:04 2017 -0700 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../db/commitlog/CommitLogReplayer.java | 11 +++++++++++ .../cassandra/db/commitlog/CommitLogTest.java | 20 ++++++++++++++++---- 3 files changed, 28 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/beb9658d/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 09e4039..2839291 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -9,6 +9,7 @@ * Fix failing COPY TO STDOUT (CASSANDRA-12497) * Fix ColumnCounter::countAll behaviour for reverse queries (CASSANDRA-13222) * Exceptions encountered calling getSeeds() breaks OTC thread (CASSANDRA-13018) + * Commitlog replay may fail if last mutation is within 4 bytes of end of segment (CASSANDRA-13282) Merged from 2.1: * Remove unused repositories (CASSANDRA-13278) * Log stacktrace of uncaught exceptions (CASSANDRA-13108) http://git-wip-us.apache.org/repos/asf/cassandra/blob/beb9658d/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java b/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java index a58aeb4..3cf4d0f 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java @@ -439,6 +439,17 @@ public class CommitLogReplayer int serializedSize; try { + // We rely on reading serialized size == 0 (LEGACY_END_OF_SEGMENT_MARKER) to identify the end + // of a segment, which happens naturally due to the 0 padding of the empty segment on creation. + // However, with 2.1 era commitlogs it's possible that the last mutation ended less than 4 bytes + // from the end of the file, which means that we'll be unable to read an a full int and instead + // read an EOF here + if(end - reader.getFilePointer() < 4) + { + logger.trace("Not enough bytes left for another mutation in this CommitLog segment, continuing"); + return false; + } + // any of the reads may hit EOF serializedSize = reader.readInt(); if (serializedSize == LEGACY_END_OF_SEGMENT_MARKER) http://git-wip-us.apache.org/repos/asf/cassandra/blob/beb9658d/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java b/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java index 9999b42..9b63885 100644 --- a/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java +++ b/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java @@ -137,12 +137,24 @@ public class CommitLogTest } @Test + public void testRecoveryWithShortPadding() throws Exception + { + // If we have 0-3 bytes remaining, commitlog replayer + // should pass, because there's insufficient room + // left in the segment for the legacy size marker. + testRecovery(new byte[1], null); + testRecovery(new byte[2], null); + testRecovery(new byte[3], null); + } + + @Test public void testRecoveryWithShortSize() throws Exception { - runExpecting(new WrappedRunnable() { - public void runMayThrow() throws Exception - { - testRecovery(new byte[2], CommitLogDescriptor.VERSION_20); + runExpecting(new WrappedRunnable() { + public void runMayThrow() throws Exception { + byte[] data = new byte[5]; + data[3] = 1; // Not a legacy marker, give it a fake (short) size + testRecovery(data, CommitLogDescriptor.VERSION_20); } }, CommitLogReplayException.class); }