You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jj...@apache.org on 2017/03/13 05:00:22 UTC
[04/10] cassandra git commit: Commitlog replay may fail if last
mutation is within 4 bytes of end of segment
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 <je...@jeffjirsa.net>
Authored: Sun Mar 12 21:54:04 2017 -0700
Committer: Jeff Jirsa <je...@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);
}