bbejeck commented on code in PR #23999:
URL: https://github.com/apache/kafka/pull/23999#discussion_r4425901665
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTransactionBuffer.java:
##########
@@ -135,16 +139,24 @@ void stageAll(final Runnable stagingOperations) {
/**
* Stages a range deletion for an explicit column family. Updates the
shared
* {@code pendingWrites} and {@code rangeTombstones} so iterators opened
before
- * commit hide the deleted range, and appends the range delete to the
shared
- * {@link WriteBatch} under {@code cf}.
+ * commit hide the deleted range, and records the range delete for {@code
cf} so
Review Comment:
```suggestion
* commits hide the deleted range, and records the range delete for
{@code cf} so
```
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTransactionBuffer.java:
##########
@@ -135,16 +139,24 @@ void stageAll(final Runnable stagingOperations) {
/**
* Stages a range deletion for an explicit column family. Updates the
shared
* {@code pendingWrites} and {@code rangeTombstones} so iterators opened
before
- * commit hide the deleted range, and appends the range delete to the
shared
- * {@link WriteBatch} under {@code cf}.
+ * commit hide the deleted range, and records the range delete for {@code
cf} so
+ * {@link #prepareFlush()} adds it to the {@link WriteBatch}.
*/
void stageDeleteRange(final ColumnFamilyHandle cf, final Bytes from, final
Bytes to) {
snapshotLock.writeLock().lock();
try {
// Multi-node subMap().clear() on the (non-thread-safe) TreeMap
staging buffer must hold
// the write lock; doing it together with the rangeTombstones swap
also makes the two
// updates atomic with respect to a non-owner snapshot scan.
- pendingWrites.subMap(from, true, to, false).clear();
+ final NavigableMap<Bytes, Optional<byte[]>> staged =
+ cf == cfHandle ? pendingWrites : otherCfWrites.get(cf);
+ if (staged != null) {
Review Comment:
I get that we need to check for `null` but I'm wondering - The fact that an
expected cf is `null` seems somewhat noteworthy - worth a log warning
statement? Maybe I'm making somethign out of nothing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
