diff --git a/CHANGES.txt b/CHANGES.txt index 302a028d8e..ab28dd4523 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,6 +34,7 @@ Merged from 3.0: * Provide user workaround when system_schema.columns does not contain entries for a table that's in system_schema.tables (CASSANDRA-13180) Merged from 2.2: + * Commitlog replay may fail if last mutation is within 4 bytes of end of segment (CASSANDRA-13282) * Fix queries updating multiple time the same list (CASSANDRA-13130) * Fix GRANT/REVOKE when keyspace isn't specified (CASSANDRA-13053) * Fix flaky LongLeveledCompactionStrategyTest (CASSANDRA-12202) diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java b/src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java index e6e2e1a53b..d1cb8d6347 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java @@ -265,6 +265,18 @@ public class CommitLogReader 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, it's possible with 2.1 era commitlogs 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"); + statusTracker.requestTermination(); + return; + } + // any of the reads may hit EOF serializedSize = reader.readInt(); if (serializedSize == LEGACY_END_OF_SEGMENT_MARKER) diff --git a/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java b/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java index 5476d032a8..4000fbf856 100644 --- a/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java +++ b/test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java @@ -170,11 +170,24 @@ public class CommitLogTest testRecoveryWithBadSizeArgument(100, 10); } + @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 { + byte[] data = new byte[5]; + data[3] = 1; // Not a legacy marker, give it a fake (short) size runExpecting(() -> { - testRecovery(new byte[2], CommitLogDescriptor.VERSION_20); + testRecovery(data, CommitLogDescriptor.VERSION_20); return null; }, CommitLogReplayException.class); }