You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "maedhroz (via GitHub)" <gi...@apache.org> on 2026/04/17 06:45:51 UTC
[PR] CASSANDRA-21255 Ensure end-of-partition bounds with no concrete positions cannot be marked as the last real clustering in SSTableCursorWriter [cassandra]
maedhroz opened a new pull request, #4748:
URL: https://github.com/apache/cassandra/pull/4748
patch by Caleb Rackliffe; reviewed by ? for CASSANDRA-21255
--
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
Re: [PR] CASSANDRA-21255 Ensure end-of-partition bounds with no concrete positions cannot be marked as the last real clustering in SSTableCursorWriter [cassandra]
Posted by "nitsanw (via GitHub)" <gi...@apache.org>.
nitsanw commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3101938408
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -554,21 +554,26 @@ public void writeRangeTombstone(UnfilteredDescriptor rangeTombstone, boolean upd
updateMetadataAndIndexBlock(rangeTombstone, unfilteredStartPosition, unfilteredEndPosition, updateClusteringMetadata);
}
- private void updateMetadataAndIndexBlock(
- UnfilteredDescriptor unfilteredDescriptor,
- long unfilteredStartPosition,
- long unfilteredEndPosition,
- boolean updateClusteringMetadata) throws IOException
+ private void updateMetadataAndIndexBlock(UnfilteredDescriptor unfilteredDescriptor,
+ long unfilteredStartPosition,
+ long unfilteredEndPosition,
+ boolean updateClusteringMetadata) throws IOException
{
if (updateClusteringMetadata) updateClusteringMetadata(unfilteredDescriptor);
// write the first clustering into rowIndexEntries buffer (we will need it unless we never write the first entry)
- if (unfilteredStartPosition == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position())) {
+ if (currentOffsetInPartition(unfilteredStartPosition) == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position()))
Review Comment:
Looks right
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -554,21 +554,26 @@ public void writeRangeTombstone(UnfilteredDescriptor rangeTombstone, boolean upd
updateMetadataAndIndexBlock(rangeTombstone, unfilteredStartPosition, unfilteredEndPosition, updateClusteringMetadata);
}
- private void updateMetadataAndIndexBlock(
- UnfilteredDescriptor unfilteredDescriptor,
- long unfilteredStartPosition,
- long unfilteredEndPosition,
- boolean updateClusteringMetadata) throws IOException
+ private void updateMetadataAndIndexBlock(UnfilteredDescriptor unfilteredDescriptor,
+ long unfilteredStartPosition,
+ long unfilteredEndPosition,
+ boolean updateClusteringMetadata) throws IOException
{
if (updateClusteringMetadata) updateClusteringMetadata(unfilteredDescriptor);
// write the first clustering into rowIndexEntries buffer (we will need it unless we never write the first entry)
- if (unfilteredStartPosition == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position())) {
+ if (currentOffsetInPartition(unfilteredStartPosition) == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position()))
+ {
writeClusteringToRowIndexEntries(unfilteredDescriptor);
}
else
{
- rowIndexEntryLastClustering.copy(unfilteredDescriptor);
+ // Only update last clustering tracking if the descriptor has actual bytes. A synthesized end-of-partition
+ // bound (e.g. INCL_END_BOUND with clusteringLength == 0) carries no real position and should not overwrite
+ // the last real clustering.
+ if (unfilteredDescriptor.clusteringLength() > 0)
+ rowIndexEntryLastClustering.copy(unfilteredDescriptor);
}
Review Comment:
AFAICT this happens unconditionally in https://github.com/apache/cassandra/blob/ff78780d6125010eb5909477c7a10a2540ca69bc/src/java/org/apache/cassandra/io/sstable/format/SortedTablePartitionWriter.java#L145
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#issuecomment-4284796513
6.0 PR is here: https://github.com/apache/cassandra/pull/4760
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "blambov (via GitHub)" <gi...@apache.org>.
blambov commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3110835506
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -596,8 +600,10 @@ private void addIndexBlock(long endOfRowPosition, long indexBlockSize) throws IO
/** {@link org.apache.cassandra.io.sstable.IndexInfo.Serializer#serialize(org.apache.cassandra.io.sstable.IndexInfo, org.apache.cassandra.io.util.DataOutputPlus)}*/
rowIndexEntriesOffsets.addInt(rowIndexEntryOffset);
- // first clustering is already in, write last entry
- if (rowIndexEntryLastClustering.clusteringLength() == 0) {
+ // First clustering is already in, so write last entry, but differentiate between only one clustering being
+ // written and a clustering with no bytes being written.
+ if (!lastClusteringEmpty && rowIndexEntryLastClustering.clusteringLength() == 0)
Review Comment:
If the condition here is to not have added a last entry, why not simplify things by making a boolean that tracks that? One that is set to true unconditionally on L572, which will also remove the need to check `clusteringLength` in both places.
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#issuecomment-4270929240
I've made a couple changes to ensure that the blocks we write in the cursor path are the same as the non-cursor path. Here's Claude's summary:
### The Bug
In the original `addIndexBlock()`:
```java
// first clustering is already in, write last entry
if (rowIndexEntryLastClustering.clusteringLength() == 0) {
// first entry is the last entry, copy it
...
} else {
writeClusteringToRowIndexEntries(rowIndexEntryLastClustering);
rowIndexEntryLastClustering.resetClustering();
}
```
`clusteringLength() == 0` was used as a sentinel meaning *"no last clustering was set, so the first clustering is also the last"*. But this conflates two distinct states:
1. **Only one unfiltered in the block** → `rowIndexEntryLastClustering` was never populated via `copy()`, so its length is 0 by default. The first entry should be copied as the last. ✓
2. **A range tombstone bound with zero clustering columns** (e.g., on a table with no clustering columns, or a bound that serializes to 0 bytes) → `clusteringLength()` is also 0, but a distinct last clustering *was* set. The first entry should **not** be copied as the last. ✗
In case 2, the original code incorrectly copies the first clustering as the last, producing a malformed `IndexInfo` entry where `firstName == lastName` even though the block spans multiple unfilteds. This corrupts the index block boundaries, causing the `IndexState` seek logic to misidentify which block to start reading from during range queries — specifically, it can skip the block containing the range tombstone bound entirely, making the open marker invisible to the reader.
### Why the Three Tests Catch This
The tests use `(pk text, ck text, ...)` — `ck` is a single `text` clustering column, so bounds like `INCL_START_BOUND(r027)` have non-zero clustering bytes. The bug wouldn't trigger on *these specific* bounds. However, the `testMidBlockRangeTombstoneInLastBlock` test writes both an `INCL_START_BOUND` and an `EXCL_END_BOUND` (or `INCL_END_BOUND`) into the last block. The sequence within that block is:
```
row r023 ← first clustering of block (written to rowIndexEntries directly)
row r024
INCL_START_BOUND(r024) ← range tombstone open
INCL_END_BOUND(r025) ← range tombstone close ← last clustering tracked in rowIndexEntryLastClustering
row r026
...
```
The `lastClusteringEmpty` flag is needed for a subtler case: when `unfilteredDescriptor.clusteringLength() == 0` is detected in the `else` branch of `updateMetadataAndIndexBlock`, it means a real unfiltered with empty clustering bytes was encountered as the last item. Without the flag, `addIndexBlock` can't distinguish this from "no last clustering set."
### The Fix — Line by Line
**1. Field declaration moved to class level (was already there in the original, but the patch shows it being added):**
```java
private boolean lastClusteringEmpty = false;
```
This is the discriminator flag.
**2. Reset in `writePartitionStart`:**
```java
lastClusteringEmpty = false;
```
Correct — must be reset per partition, not just per index block, since `addIndexBlock` resets it at the end of each block call.
**3. Set the flag in `updateMetadataAndIndexBlock`:**
```java
else
{
lastClusteringEmpty = unfilteredDescriptor.clusteringLength() == 0;
rowIndexEntryLastClustering.copy(unfilteredDescriptor);
}
```
When the last-seen unfiltered has zero clustering bytes, record that fact separately from the clustering data itself.
**4. The guard in `addIndexBlock`:**
```java
if (!lastClusteringEmpty && rowIndexEntryLastClustering.clusteringLength() == 0)
{
// first entry is the last entry, copy it
...
}
else
{
writeClusteringToRowIndexEntries(rowIndexEntryLastClustering);
rowIndexEntryLastClustering.resetClustering();
}
lastClusteringEmpty = false;
```
The condition now correctly means: *"the last clustering was never set (only one unfiltered in this block) AND it's not the case that we explicitly saw a zero-length clustering"*. The `lastClusteringEmpty = false` reset at the end is correct — it's scoped per index block.
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3102685003
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -554,21 +554,26 @@ public void writeRangeTombstone(UnfilteredDescriptor rangeTombstone, boolean upd
updateMetadataAndIndexBlock(rangeTombstone, unfilteredStartPosition, unfilteredEndPosition, updateClusteringMetadata);
}
- private void updateMetadataAndIndexBlock(
- UnfilteredDescriptor unfilteredDescriptor,
- long unfilteredStartPosition,
- long unfilteredEndPosition,
- boolean updateClusteringMetadata) throws IOException
+ private void updateMetadataAndIndexBlock(UnfilteredDescriptor unfilteredDescriptor,
+ long unfilteredStartPosition,
+ long unfilteredEndPosition,
+ boolean updateClusteringMetadata) throws IOException
{
if (updateClusteringMetadata) updateClusteringMetadata(unfilteredDescriptor);
// write the first clustering into rowIndexEntries buffer (we will need it unless we never write the first entry)
- if (unfilteredStartPosition == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position())) {
+ if (currentOffsetInPartition(unfilteredStartPosition) == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position()))
+ {
writeClusteringToRowIndexEntries(unfilteredDescriptor);
}
else
{
- rowIndexEntryLastClustering.copy(unfilteredDescriptor);
+ // Only update last clustering tracking if the descriptor has actual bytes. A synthesized end-of-partition
+ // bound (e.g. INCL_END_BOUND with clusteringLength == 0) carries no real position and should not overwrite
+ // the last real clustering.
+ if (unfilteredDescriptor.clusteringLength() > 0)
+ rowIndexEntryLastClustering.copy(unfilteredDescriptor);
}
Review Comment:
Yeah, even though this passes the test, I think it is actually causing a different last clustering to be written in the third block. Looking at another angle where I fix in `addIndexBlock()` instead...
--
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
Re: [PR] CASSANDRA-21255 Ensure end-of-partition bounds with no concrete positions cannot be marked as the last real clustering in SSTableCursorWriter [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3102685003
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -554,21 +554,26 @@ public void writeRangeTombstone(UnfilteredDescriptor rangeTombstone, boolean upd
updateMetadataAndIndexBlock(rangeTombstone, unfilteredStartPosition, unfilteredEndPosition, updateClusteringMetadata);
}
- private void updateMetadataAndIndexBlock(
- UnfilteredDescriptor unfilteredDescriptor,
- long unfilteredStartPosition,
- long unfilteredEndPosition,
- boolean updateClusteringMetadata) throws IOException
+ private void updateMetadataAndIndexBlock(UnfilteredDescriptor unfilteredDescriptor,
+ long unfilteredStartPosition,
+ long unfilteredEndPosition,
+ boolean updateClusteringMetadata) throws IOException
{
if (updateClusteringMetadata) updateClusteringMetadata(unfilteredDescriptor);
// write the first clustering into rowIndexEntries buffer (we will need it unless we never write the first entry)
- if (unfilteredStartPosition == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position())) {
+ if (currentOffsetInPartition(unfilteredStartPosition) == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position()))
+ {
writeClusteringToRowIndexEntries(unfilteredDescriptor);
}
else
{
- rowIndexEntryLastClustering.copy(unfilteredDescriptor);
+ // Only update last clustering tracking if the descriptor has actual bytes. A synthesized end-of-partition
+ // bound (e.g. INCL_END_BOUND with clusteringLength == 0) carries no real position and should not overwrite
+ // the last real clustering.
+ if (unfilteredDescriptor.clusteringLength() > 0)
+ rowIndexEntryLastClustering.copy(unfilteredDescriptor);
}
Review Comment:
Yeah, even though this passes the test, I think it is actually causing a different last clustering to be written in the `IndexInfo`. Looking at another angle where I fix in `addIndexBlock()` instead...
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3102998581
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -554,21 +554,26 @@ public void writeRangeTombstone(UnfilteredDescriptor rangeTombstone, boolean upd
updateMetadataAndIndexBlock(rangeTombstone, unfilteredStartPosition, unfilteredEndPosition, updateClusteringMetadata);
}
- private void updateMetadataAndIndexBlock(
- UnfilteredDescriptor unfilteredDescriptor,
- long unfilteredStartPosition,
- long unfilteredEndPosition,
- boolean updateClusteringMetadata) throws IOException
+ private void updateMetadataAndIndexBlock(UnfilteredDescriptor unfilteredDescriptor,
+ long unfilteredStartPosition,
+ long unfilteredEndPosition,
+ boolean updateClusteringMetadata) throws IOException
{
if (updateClusteringMetadata) updateClusteringMetadata(unfilteredDescriptor);
// write the first clustering into rowIndexEntries buffer (we will need it unless we never write the first entry)
- if (unfilteredStartPosition == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position())) {
+ if (currentOffsetInPartition(unfilteredStartPosition) == indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position()))
+ {
writeClusteringToRowIndexEntries(unfilteredDescriptor);
}
else
{
- rowIndexEntryLastClustering.copy(unfilteredDescriptor);
+ // Only update last clustering tracking if the descriptor has actual bytes. A synthesized end-of-partition
+ // bound (e.g. INCL_END_BOUND with clusteringLength == 0) carries no real position and should not overwrite
+ // the last real clustering.
+ if (unfilteredDescriptor.clusteringLength() > 0)
+ rowIndexEntryLastClustering.copy(unfilteredDescriptor);
}
Review Comment:
UPDATE: Did this. Full summary below.
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3112416756
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -596,8 +600,10 @@ private void addIndexBlock(long endOfRowPosition, long indexBlockSize) throws IO
/** {@link org.apache.cassandra.io.sstable.IndexInfo.Serializer#serialize(org.apache.cassandra.io.sstable.IndexInfo, org.apache.cassandra.io.util.DataOutputPlus)}*/
rowIndexEntriesOffsets.addInt(rowIndexEntryOffset);
- // first clustering is already in, write last entry
- if (rowIndexEntryLastClustering.clusteringLength() == 0) {
+ // First clustering is already in, so write last entry, but differentiate between only one clustering being
+ // written and a clustering with no bytes being written.
+ if (!lastClusteringEmpty && rowIndexEntryLastClustering.clusteringLength() == 0)
Review Comment:
@blambov How about this?
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3111982138
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -596,8 +600,10 @@ private void addIndexBlock(long endOfRowPosition, long indexBlockSize) throws IO
/** {@link org.apache.cassandra.io.sstable.IndexInfo.Serializer#serialize(org.apache.cassandra.io.sstable.IndexInfo, org.apache.cassandra.io.util.DataOutputPlus)}*/
rowIndexEntriesOffsets.addInt(rowIndexEntryOffset);
- // first clustering is already in, write last entry
- if (rowIndexEntryLastClustering.clusteringLength() == 0) {
+ // First clustering is already in, so write last entry, but differentiate between only one clustering being
+ // written and a clustering with no bytes being written.
+ if (!lastClusteringEmpty && rowIndexEntryLastClustering.clusteringLength() == 0)
Review Comment:
Let me see if that works...
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz closed pull request #4748: CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock()
URL: https://github.com/apache/cassandra/pull/4748
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#issuecomment-4290178454
Committed as https://github.com/apache/cassandra/commit/fb4178f4482a1e6a071e487a20aac60eea9cd52e
--
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
Re: [PR] CASSANDRA-21255 Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() [cassandra]
Posted by "blambov (via GitHub)" <gi...@apache.org>.
blambov commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3115639267
##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -596,8 +600,10 @@ private void addIndexBlock(long endOfRowPosition, long indexBlockSize) throws IO
/** {@link org.apache.cassandra.io.sstable.IndexInfo.Serializer#serialize(org.apache.cassandra.io.sstable.IndexInfo, org.apache.cassandra.io.util.DataOutputPlus)}*/
rowIndexEntriesOffsets.addInt(rowIndexEntryOffset);
- // first clustering is already in, write last entry
- if (rowIndexEntryLastClustering.clusteringLength() == 0) {
+ // First clustering is already in, so write last entry, but differentiate between only one clustering being
+ // written and a clustering with no bytes being written.
+ if (!lastClusteringEmpty && rowIndexEntryLastClustering.clusteringLength() == 0)
Review Comment:
Yes, this looks good to me.
--
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