krummas commented on a change in pull request #1180:
URL: https://github.com/apache/cassandra/pull/1180#discussion_r711021560
##########
File path: src/java/org/apache/cassandra/db/ReadCommand.java
##########
@@ -632,6 +652,83 @@ private void maybeDelayForTesting()
}
}
+ private UnfilteredPartitionIterator
withQuerySizeTracking(UnfilteredPartitionIterator iterator)
+ {
+ if (!trackWarnings ||
SchemaConstants.isSystemKeyspace(metadata().keyspace)) // exclude internal
keyspaces
+ return iterator;
+ final long warnThresholdBytes =
DatabaseDescriptor.getLocalReadSizeWarnThresholdKb() * 1024;
+ final long abortThresholdBytes =
DatabaseDescriptor.getLocalReadSizeAbortThresholdKb() * 1024;
+ if (warnThresholdBytes == 0 && abortThresholdBytes == 0)
Review comment:
nit; maybe create a "shouldTrackSize()" method and fold up this with the
if statement above
##########
File path: src/java/org/apache/cassandra/db/MessageParams.java
##########
@@ -50,6 +50,14 @@ public static void add(ParamType key, Object value)
get().put(key, value);
}
+ public static <T extends Comparable<T>> void addIfLarger(ParamType key, T
value)
Review comment:
nit; this feels quite specific to this use case - maybe add a `addIf`
method which takes a predicate?
##########
File path: src/java/org/apache/cassandra/db/MessageParams.java
##########
@@ -50,6 +50,14 @@ public static void add(ParamType key, Object value)
get().put(key, value);
}
+ public static <T extends Comparable<T>> void addIfLarger(ParamType key, T
value)
Review comment:
nit; this feels too specific - maybe make it `addIf` which takes a
Predicate?
##########
File path: src/java/org/apache/cassandra/db/RowIndexEntry.java
##########
@@ -343,6 +349,47 @@ public static void skipForCache(DataInputPlus in) throws
IOException
}
}
+ private void checkSize(int entries, int bytes)
+ {
+ ReadCommand command = ReadCommand.getCommand();
+ if (command == null ||
SchemaConstants.isSystemKeyspace(command.metadata().keyspace) ||
!DatabaseDescriptor.getTrackWarningsEnabled())
+ return;
+
+ int warnThreshold =
DatabaseDescriptor.getRowIndexSizeWarnThresholdKb() * 1024;
+ int abortThreshold =
DatabaseDescriptor.getRowIndexSizeAbortThresholdKb() * 1024;
Review comment:
return early if `warnThreshold` and `abortThreshold` are both 0?
##########
File path: src/java/org/apache/cassandra/service/reads/ReadCallback.java
##########
@@ -193,9 +228,32 @@ else if (logger.isDebugEnabled())
logger.debug("{}; received {} of {} responses{}", failed ?
"Failed" : "Timed out", received, blockFor, gotData);
}
- if (warnings != null && warnings.tombstoneAborts.get() > 0)
- throw new TombstoneAbortException(warnings.tombstoneAborts.get(),
warnings.maxTombstoneAbortsCount.get(), command.toCQLString(),
resolver.isDataPresent(),
-
replicaPlan.get().consistencyLevel(), received, blockFor,
failureReasonByEndpoint);
+ if (snapshot != null)
Review comment:
nit; maybe move all this in to `WarningsSnapshot` to keep this method a
bit smaller/cleaner - add something like `snapshot.maybeAbort(...)`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
