CNDB-15946: Revamp SAI metrics for fetched/returned keys/partitions/rows/tombstones

[adelapena]

What is the issue

The metrics partitionsRead, rowsFiltered, rowsPreFiltered and shadowedPrimaryKeyCount don't always work as expected. Details on https://github.com/riptano/cndb/issues/15946.

What does this PR fix and why was it fixed

Replace those metrics with:

• keysFetched: Number of partition/row keys that will be used to fetch rows from the base table.
• partitionsFetched: Number of live partitions fetched from the base table, before post-filtering and sorting. Note that currently ANN fetches one partition per index key, without any grouping of same-partition clusterings.
• partitionsReturned: Number of live partitions returned to the coordinator, after post-filtering and sorting.
• partitionTombstonesFetched: Number of deleted partitions found when reading the base table.
• rowsFetched: Number of live rows fetched from the base table, before post-filtering and sorting.
• rowsReturned: Number of live rows returned to the coordinator, after post-filtering and sorting.
• rowTombstonesFetched: Number deleted rows or row ranges found when reading the base table.

StorageAttachedIndexSearcher is modified to use the command timestamp, rather than getting the current
time everytime a query timestamp is needed, which was possibly buggy and inefficient.

Here are some examples on how to interpret these metrics for AA partition-aware disk format:

• keysFetched > partitionsFetched: The indexed data has changed since it was indexed and there has been deletions or updates.
• partitionsFetched < rowsFetched: The partitions are wide, if the difference is large queries with high within-partition selectivity might behave poorly compared to later disk formats, and vice versa.
• rowsFetched > rowsReturned: The queries have some ALLOW FILTERING restriction or the partition-aware nature of the index is producing filtering.

And for row-aware disk formats later than AA :

• keysFetched > rowsFetched: The indexed data has changed since it was indexed and there has been deletions or updates.
• partitionsFetched < rowsFetched: The partitions are wide, if the difference is large queries with low within-partition selectivity might behave worse than with AA, and vice versa.
• rowsFetched > rowsReturned: The queries have some ALLOW FILTERING restriction that is producing filtering.

[github-actions]

Checklist before you submit for review

• This PR adheres to the Definition of Done
• Make sure there is a PR in the CNDB project updating the Converged Cassandra version
• Use NoSpamLogger for log lines that may appear frequently in the logs
• Verify test results on Butler
• Test coverage for new/modified code is > 80%
• Proper code formatting
• Proper title for each commit staring with the project-issue number, like CNDB-1234
• Each commit has a meaningful description
• Each commit is not very long and contains related changes
• Renames, moves and reformatting are in distinct commits
• All new files should contain the DataStax copyright header instead of the Apache License one

[adelapena]

The three test failures above are junit timeouts, probably unrelated since those tests take quite long in main and we are having trouble with timeouts.

[ekaterinadimitrova2]

Still need to finish reviewing some of the tests, you added so many, that's great! But I wanted to push my first comments and questions for consideration. I am a bit nervous around the transformations' changes, (reason - I haven't done a lot in that area of the code) so I am trying to be very carefully reasoning about all the cases.

[pkolaczk]

Great job! Those new metric names make a lot more sense.
Much more readable and complete than before!
Thank you! :)

[adelapena]

Looks like the only new CI failures are unrelated timeouts.

[ekaterinadimitrova2]

Between QueryContextTest and SlowSAIQueryLoggerTest, I was wondering what other scenarios we may want to add. The only thing I can think of are collections and TTL. WDYT? Does it make sense to add a few small tests?

Also, I think it is quite surprising with the current documentation and naming how partitionsFetched works with ANN queries. It is confusing for users. I guess changing the metric for ANN queries is hard but then we should change name/document better? Thoughts?

CC @pkolaczk

The rest looks great!

[adelapena]

Between QueryContextTest and SlowSAIQueryLoggerTest, I was wondering what other scenarios we may want to add. The only thing I can think of are collections and TTL. WDYT? Does it make sense to add a few small tests?

I don't see lots of value, but why not. I have added tests to QueryContextTest only, because SlowSAIQueryLoggerTest and QueryMetricsTest only need to check that the QueryContext metrics get published in logs and query metrics. We don't need to test again there the various ways in which those metrics can get their values.

[ekaterinadimitrova2]

Great job fixing and testing these metrics, thanks.
I see CI errored-out so the merge is pending on clean CI and approval of the other PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cassci-bot]

❌ Build ds-cassandra-pr-gate/PR-2132 rejected by Butler

---

2 regressions found
See build details here

---

Found 2 new test failures

Test	Explanation	Runs	Upstream
o.a.c.index.sai.cql.VectorCompaction100dTest.testCompactionWithEnoughRowsForPQAndDeleteARow[eb false]	REGRESSION	🔴⚪	0 / 18
o.a.c.index.sai.cql.VectorSiftSmallTest.testSiftSmall[dc false]	REGRESSION	🔴⚪	0 / 18

---

Found 4 known test failures

[adelapena]

The only new test failures are unrelated junit timeouts, so CI looks good to me.