CASSANDRA-13001 Implement appender of slow queries to system_views.slow_queries table #4067

smiklosovic · 2025-04-09T11:42:29Z

patch by Stefan Miklosovic; reviewed by TBD for CASSANDRA-13001

Thanks for sending a pull request! Here are some tips if you're new here:

Ensure you have added or run the appropriate tests for your PR.
Be sure to keep the PR description updated to reflect all changes.
Write your PR title to summarize what this PR proposes.
If possible, provide a concise example to reproduce the issue for a faster review.
Read our contributor guidelines
If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

src/java/org/apache/cassandra/db/monitoring/MonitoringTask.java

bbotella · 2025-04-10T14:26:54Z

src/java/org/apache/cassandra/db/virtual/AbstractLoggerVirtualTable.java

+    @Override
+    public void truncate()
+    {
+        buffer.clear();


Is it worth adding a debug logger message stating that the table has been truncated?

I dont think so, really.

bbotella · 2025-04-10T14:30:57Z

src/java/org/apache/cassandra/utils/logging/AbstractVirtualTableAppender.java

+    {
+        // we restrict how many logging events we can put into buffer,
+        // so we are not growing without any bound when things go south
+        if (messageBuffer.size() < defaultRows)


Should we add a logger debug if message is not added to the buffer due to buffer size?

That would most probably cause a flood of debug messages every time we are "over the limit" unless the logger is no-spamming one. I would keep it like it is.

bbotella · 2025-04-10T14:34:02Z

test/unit/org/apache/cassandra/db/virtual/AbstractLoggerVirtualTableTest.java

+
+        execute(query("truncate %s"));
+
+        assertTrue(executeNet(query("select timestamp from %s")).all().isEmpty());


Can we use assertEmpty?

I don't see any simple way to do that because executeNet returns ResultSet from driver but assertEmpty accepts UntypedResultSet

src/java/org/apache/cassandra/db/monitoring/MonitoringTask.java

src/java/org/apache/cassandra/db/virtual/SlowQueriesTable.java

src/java/org/apache/cassandra/utils/logging/AbstractVirtualTableAppender.java

src/java/org/apache/cassandra/db/virtual/AbstractLoggerVirtualTable.java

src/java/org/apache/cassandra/db/virtual/SlowQueriesTable.java

src/java/org/apache/cassandra/utils/logging/SlowQueriesAppender.java

src/java/org/apache/cassandra/db/monitoring/MonitoringTask.java

src/java/org/apache/cassandra/db/virtual/SlowQueriesTable.java

patch by Stefan Miklosovic; reviewed by Dmitry Konstantinov, Bernardo Botella for CASSANDRA-13001

smiklosovic requested a review from bschoening April 9, 2025 11:42

smiklosovic force-pushed the CASSANDRA-13001 branch 7 times, most recently from a794d28 to 0437269 Compare April 9, 2025 13:02

bbotella reviewed Apr 10, 2025

View reviewed changes

netudima reviewed Apr 18, 2025

View reviewed changes

src/java/org/apache/cassandra/db/monitoring/MonitoringTask.java Outdated Show resolved Hide resolved

netudima reviewed Apr 18, 2025

View reviewed changes

src/java/org/apache/cassandra/db/monitoring/MonitoringTask.java Outdated Show resolved Hide resolved

netudima reviewed Apr 18, 2025

View reviewed changes

src/java/org/apache/cassandra/db/monitoring/MonitoringTask.java Outdated Show resolved Hide resolved

netudima reviewed Apr 18, 2025

View reviewed changes

src/java/org/apache/cassandra/db/virtual/SlowQueriesTable.java Outdated Show resolved Hide resolved

netudima reviewed Apr 18, 2025

View reviewed changes

src/java/org/apache/cassandra/utils/logging/AbstractVirtualTableAppender.java Show resolved Hide resolved