You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/08/24 05:53:15 UTC
[GitHub] [cassandra] maedhroz opened a new pull request #1164: CASSANDRA-16842 Allow CommitLogSegmentReader to optionally skip sync marker CRC checks
maedhroz opened a new pull request #1164:
URL: https://github.com/apache/cassandra/pull/1164
patch by Caleb Rackliffe; reviewed by Josh McKenzie for CASSANDRA-16842
Co-authored-by: Jordan West <jo...@gmail.com>
Co-authored-by: Caleb Rackliffe <ca...@gmail.com>
Co-authored-by: Marcus Eriksson <ma...@apache.org>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] josh-mckenzie commented on a change in pull request #1164: CASSANDRA-16842 Allow CommitLogSegmentReader to optionally skip sync marker CRC checks
Posted by GitBox <gi...@apache.org>.
josh-mckenzie commented on a change in pull request #1164:
URL: https://github.com/apache/cassandra/pull/1164#discussion_r695773060
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogSegmentReader.java
##########
@@ -151,8 +165,22 @@ private int readSyncMarker(CommitLogDescriptor descriptor, int offset, RandomAcc
updateChecksumInt(crc, (int) reader.getPosition());
final int end = reader.readInt();
long filecrc = reader.readInt() & 0xffffffffL;
+
if (crc.getValue() != filecrc)
{
+ // The next marker position and CRC value are not writen atomically, so it is possible for the latter to
+ // still be zero after the former has been finalized, even though the mutations that follow it are valid.
+ // When there is no compression or encryption enabled, we can ignore a sync marker CRC mismatch and defer
+ // to the per-mutation CRCs, which may be preferable to preventing startup altogether.
+ if (allowSkipSyncMarkerCrc
+ && descriptor.compression == null && !descriptor.getEncryptionContext().isEnabled()
+ && filecrc == 0 && end != 0)
+ {
+ logger.warn("Skipping sync marker CRC check at position {} (end={}, calculated crc={}) of commit log {}...",
Review comment:
Consider adding something to the log about "Using per mutation CRC checks to ensure correctness instead"
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogSegmentReader.java
##########
@@ -151,8 +165,22 @@ private int readSyncMarker(CommitLogDescriptor descriptor, int offset, RandomAcc
updateChecksumInt(crc, (int) reader.getPosition());
final int end = reader.readInt();
long filecrc = reader.readInt() & 0xffffffffL;
+
if (crc.getValue() != filecrc)
{
+ // The next marker position and CRC value are not writen atomically, so it is possible for the latter to
Review comment:
nit: written
##########
File path: test/unit/org/apache/cassandra/db/commitlog/CommitLogTest.java
##########
@@ -764,6 +782,75 @@ public void replaySimple() throws IOException
assertEquals(cellCount, replayer.cells);
}
+ @Test
+ public void replayWithBadSyncMarkerCRC() throws IOException
+ {
+ ColumnFamilyStore cfs = Keyspace.open(KEYSPACE1).getColumnFamilyStore(STANDARD1);
+
+ Mutation rm2 = new RowUpdateBuilder(cfs.metadata(), 0, "k2").clustering("bytes")
+ .add("val", bytes("this is a string"))
+ .build();
+ CommitLog.instance.add(rm2);
+ CommitLog.instance.sync(true);
+
+ List<String> activeSegments = CommitLog.instance.getActiveSegmentNames();
+ assertFalse(activeSegments.isEmpty());
+
+ File directory = new File(CommitLog.instance.segmentManager.storageDirectory);
+ File firstActiveFile = Objects.requireNonNull(directory.listFiles((file, name) -> activeSegments.contains(name)))[0];
+ zeroFirstSyncMarkerCRC(firstActiveFile);
+
+ CommitLogSegmentReader.setAllowSkipSyncMarkerCrc(true);
+
+
Review comment:
nit: extra line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] maedhroz closed pull request #1164: CASSANDRA-16842 Allow CommitLogSegmentReader to optionally skip sync marker CRC checks
Posted by GitBox <gi...@apache.org>.
maedhroz closed pull request #1164:
URL: https://github.com/apache/cassandra/pull/1164
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] maedhroz commented on pull request #1164: CASSANDRA-16842 Allow CommitLogSegmentReader to optionally skip sync marker CRC checks
Posted by GitBox <gi...@apache.org>.
maedhroz commented on pull request #1164:
URL: https://github.com/apache/cassandra/pull/1164#issuecomment-905851992
Committed as https://github.com/apache/cassandra/commit/f9b7c1e6984f5b81aae1e3a2191d4e9599db15ae
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org