Skip to content

Commit 0bcd5ce

Browse files
author
Nitsan Wakart
committed
Bug fix: NPE on mergeCells edge case
1 parent 9d9982b commit 0bcd5ce

File tree

3 files changed

+13
-12
lines changed

3 files changed

+13
-12
lines changed

src/java/org/apache/cassandra/db/compaction/CursorCompactor.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,7 @@ private void skipRowsOnStrictLiveness(int rowMergeLimit, boolean isStatic) throw
619619

620620
private DataOutputBuffer tempCellBuffer1 = new DataOutputBuffer();
621621
private DataOutputBuffer tempCellBuffer2 = new DataOutputBuffer();
622+
private final byte[] copyColumnValueBuffer = new byte[4096]; // used to copy cell contents (maybe piecemeal if very large, since we don't have a direct read option)
622623

623624
/**
624625
* {@link Row.Merger.ColumnDataReducer#getReduced()} <-- applied the delete before reconcile, should not make a difference?
@@ -670,7 +671,9 @@ else if (cellResolution == RIGHT) {
670671
if (tempCellBuffer != null)
671672
throw new IllegalStateException("tempCellBuffer should be null if cellSource has a value to be read.");
672673
tempCellBuffer1.clear();
673-
ssTableCursorWriter.copyCellValue(cellSource, tempCellBuffer1);
674+
675+
cellSource.copyCellValue(tempCellBuffer1, copyColumnValueBuffer);
676+
674677
tempCellBuffer = tempCellBuffer1; // assume cell1 is going to be bigger
675678
}
676679
else if (tempCellBuffer == null) {
@@ -681,7 +684,8 @@ else if (tempCellBuffer != tempCellBuffer1) {
681684
throw new IllegalStateException("tempCellBuffer should be tempCellBuffer1 if cellSource has been read.");
682685
}
683686
tempCellBuffer2.clear();
684-
if (oCellSource.state() == CELL_VALUE_START) ssTableCursorWriter.copyCellValue(oCellSource, tempCellBuffer2);
687+
if (oCellSource.state() == CELL_VALUE_START)
688+
oCellSource.copyCellValue(tempCellBuffer2, copyColumnValueBuffer);
685689

686690
int compare = Arrays.compareUnsigned(tempCellBuffer1.getData(), 0, tempCellBuffer1.getLength(), tempCellBuffer2.getData(), 0, tempCellBuffer2.getLength());
687691
if (compare >= 0) {
@@ -774,7 +778,7 @@ else if (tempCellBuffer != null) {
774778
if (cellSource.state() == CELL_VALUE_START)
775779
{
776780
if (tempCellBuffer != null) throw new IllegalStateException("Either copied buffer or ready to copy reader, not both.");
777-
ssTableCursorWriter.writeCellValue(cellSource);
781+
ssTableCursorWriter.writeCellValue(cellSource, copyColumnValueBuffer);
778782
}
779783
else if (tempCellBuffer != null)
780784
{

src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ public class SSTableCursorWriter implements AutoCloseable
7979
// ROW contents, needed because of the order of writing and the var int fields
8080
private int rowFlags; // discovered as we go along
8181
private int rowExtendedFlags;
82-
private final byte[] copyColumnValueBuffer = new byte[4096]; // used to copy cell contents (maybe piecemiel if very large, since we don't have a direct read option)
8382
private final DataOutputBuffer rowHeaderBuffer = new DataOutputBuffer(); // holds the contents between FLAGS and SIZE
8483
private final DataOutputBuffer rowBuffer = new DataOutputBuffer();
8584
private final ReusableDeletionTime openMarker = ReusableDeletionTime.live();
@@ -415,14 +414,9 @@ private void writeCellHeader(int cellFlags, ReusableLivenessInfo cellLiveness, D
415414
metadataCollector.updateCellLiveness(cellLiveness);
416415
}
417416

418-
public int writeCellValue(SSTableCursorReader cursor) throws IOException
417+
public int writeCellValue(SSTableCursorReader cursor, byte[] copyColumnValueBuffer) throws IOException
419418
{
420-
return copyCellValue(cursor, rowBuffer);
421-
}
422-
423-
public int copyCellValue(SSTableCursorReader cursor, DataOutputBuffer dataOutputBuffer) throws IOException
424-
{
425-
return cursor.copyCellValue(dataOutputBuffer, copyColumnValueBuffer);
419+
return cursor.copyCellValue(rowBuffer, copyColumnValueBuffer);
426420
}
427421

428422
public void writeCellValue(DataOutputBuffer tempCellBuffer) throws IOException

test/microbench/org/apache/cassandra/test/microbench/sstable/SSTableCursorPipeUtil.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
public class SSTableCursorPipeUtil
3535
{
36+
3637
public static void copySSTable(SSTableCursorReader reader, SSTableCursorWriter writer) throws Throwable
3738
{
3839
PartitionDescriptor pHeader = new PartitionDescriptor(reader.ssTableReader().getPartitioner().createReusableKey(0));
@@ -108,6 +109,8 @@ public static int copyRangeTombstone(SSTableCursorReader reader, SSTableCursorWr
108109
return readerState;
109110
}
110111

112+
private final static byte[] copyColumnValueBuffer = new byte[4096]; // used to copy cell contents (maybe piecemeal if very large, since we don't have a direct read option)
113+
111114
public static int copyRowAfterDescriptor(SSTableCursorReader reader, SSTableCursorWriter writer, UnfilteredDescriptor unfilteredDescriptor, int readerState, boolean isStatic, boolean updateClusteringMetadata) throws IOException
112115
{
113116
writer.writeRowStart(unfilteredDescriptor.livenessInfo(), unfilteredDescriptor.deletionTime(), isStatic);
@@ -128,7 +131,7 @@ public static int copyRowAfterDescriptor(SSTableCursorReader reader, SSTableCurs
128131
writer.writeCellHeader(cellFlags, cellLiveness, cellCursor.cellColumn);
129132
if (readerState == CELL_VALUE_START)
130133
{
131-
readerState = writer.writeCellValue(reader);
134+
readerState = writer.writeCellValue(reader, copyColumnValueBuffer);
132135
}
133136
else if (Cell.Serializer.hasValue(cellFlags))
134137
{

0 commit comments

Comments
 (0)