tolbertam commented on code in PR #3917:
URL: https://github.com/apache/cassandra/pull/3917#discussion_r2045597854
##########
src/java/org/apache/cassandra/db/SystemKeyspace.java:
##########
@@ -1863,16 +1864,38 @@ public static void resetPreparedStatements()
}
public static int loadPreparedStatements(TriFunction<MD5Digest, String,
String, Boolean> onLoaded)
+ {
+ return loadPreparedStatements(onLoaded,
QueryProcessor.PRELOAD_PREPARED_STATEMENTS_FETCH_SIZE);
+ }
+
+ public static int loadPreparedStatements(TriFunction<MD5Digest, String,
String, Boolean> onLoaded, int pageSize)
{
String query = String.format("SELECT prepared_id, logged_keyspace,
query_string FROM %s.%s", SchemaConstants.SYSTEM_KEYSPACE_NAME,
PREPARED_STATEMENTS);
- UntypedResultSet resultSet = executeOnceInternal(query);
+ UntypedResultSet resultSet = executeOnceInternalWithPaging(query,
pageSize);
int counter = 0;
+ long initialStatementsEvicted =
QueryProcessor.metrics.preparedStatementsEvicted.getCount();
for (UntypedResultSet.Row row : resultSet)
{
if
(onLoaded.accept(MD5Digest.wrap(row.getByteArray("prepared_id")),
row.getString("query_string"),
row.has("logged_keyspace") ?
row.getString("logged_keyspace") : null))
counter++;
+
+ if (counter % pageSize == 0
+ && QueryProcessor.metrics.preparedStatementsEvicted.getCount()
- initialStatementsEvicted > 0)
Review Comment:
Was just thinking this through and I was cautiously worried about two things:
1. Incidentally warning the user that they might have hit a leak, when they
really haven't (e.g. if cache was full on restart and we detected it was full
and we 5% wasn't enough of a buffer)
2. Not detecting the cache is full because 5% is too big of a buffer
But I just realized that one thing we can use in our favor is that we only
evaluate this on paging boundaries, so its exceedingly unlikely that either of
those 2 cases would be a problem with a page size of 5000. Therefore, I think
it's ok to do this, and that we also don't need a 5% buffer, as evaluating on
page boundaries acts as a buffer in itself. Will push something shortly that
will should work I hope.
##########
src/java/org/apache/cassandra/db/SystemKeyspace.java:
##########
@@ -1863,16 +1864,38 @@ public static void resetPreparedStatements()
}
public static int loadPreparedStatements(TriFunction<MD5Digest, String,
String, Boolean> onLoaded)
+ {
+ return loadPreparedStatements(onLoaded,
QueryProcessor.PRELOAD_PREPARED_STATEMENTS_FETCH_SIZE);
+ }
+
+ public static int loadPreparedStatements(TriFunction<MD5Digest, String,
String, Boolean> onLoaded, int pageSize)
{
String query = String.format("SELECT prepared_id, logged_keyspace,
query_string FROM %s.%s", SchemaConstants.SYSTEM_KEYSPACE_NAME,
PREPARED_STATEMENTS);
- UntypedResultSet resultSet = executeOnceInternal(query);
+ UntypedResultSet resultSet = executeOnceInternalWithPaging(query,
pageSize);
int counter = 0;
+ long initialStatementsEvicted =
QueryProcessor.metrics.preparedStatementsEvicted.getCount();
for (UntypedResultSet.Row row : resultSet)
{
if
(onLoaded.accept(MD5Digest.wrap(row.getByteArray("prepared_id")),
row.getString("query_string"),
row.has("logged_keyspace") ?
row.getString("logged_keyspace") : null))
counter++;
+
+ if (counter % pageSize == 0
+ && QueryProcessor.metrics.preparedStatementsEvicted.getCount()
- initialStatementsEvicted > 0)
Review Comment:
Was just thinking this through and I was cautiously worried about two things:
1. Incidentally warning the user that they might have hit a leak, when they
really haven't (e.g. if cache was full on restart and we detected it was full
and we 5% wasn't enough of a buffer)
2. Not detecting the cache is full because 5% is too big of a buffer and
paging indefinitely.
But I just realized that one thing we can use in our favor is that we only
evaluate this on paging boundaries, so its exceedingly unlikely that either of
those 2 cases would be a problem with a page size of 5000. Therefore, I think
it's ok to do this, and that we also don't need a 5% buffer, as evaluating on
page boundaries acts as a buffer in itself. Will push something shortly that
will should work I hope.
--
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-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
