CNDB-15620: Fix sstablesHit and segmentsHit metrics to be updated by ANN and generic OrderBy

[ekaterinadimitrova2]

…

What is the issue

...
SAI sstablesHit and segmentsHit metrics are not updated by ANN and generic ORDER BY

What does this PR fix and why was it fixed

...
Fix sstablesHit and segmentsHit metrics to be updated by ANN and generic OrderBy

[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

[ekaterinadimitrova2]

This is ready for review, the test coverage complains about two assertions not being covered, but that would be a bug, so I think we are good from test coverage perspective.

The only test failure has actually a ticket for the same one. It does not fail locally and I keep on seeing it also on other PRs today.

[adelapena]

Why do we need to checkpoint? Not saying we don't, just asking.

[ekaterinadimitrova2]

My understanding is we do it because if the query already timed out, we do not need really to update the metrics.
Also, the same approach was taken with the place where we were already updating the metric, so I think we should also stay consistent. Does this make sense?

[adelapena]

This doesn't seem hit by any query in SlowSAIQueryLoggerTest, the test passes without this change.

[adelapena]

It can be validated with:

// Disable query optimizer to prevent skipping hybrid query logic.
node.runOnInstance(() -> QueryController.QUERY_OPT_LEVEL = 0);
long mark = node.logs().mark();
coordinator.execute(hybridQuery, ConsistencyLevel.ONE);
assertLogsContain(mark, node,
                  "SAI slow query metrics:",
                  "sstablesHit: 2",
                  "segmentsHit: 2",
                  "partitionsRead: 3",
                  "rowsFiltered: 3",
                  "rowsPreFiltered: 0",
                  "trieSegmentsHit: 0",
                  "bkdPostingListsHit: 1",
                  "bkdSegmentsHit: 1",
                  "bkdPostingsSkips: 0",
                  "bkdPostingsDecodes: 4",
                  "triePostingsSkips: 0",
                  "triePostingsDecodes: 0",
                  "annGraphSearchLatencyNanos: 0",
                  "shadowedPrimaryKeyCount: 0",
                  "SAI slow query plan:",
                  "KeysSort",
                  "NumericIndexScan");

[ekaterinadimitrova2]

Interesting, I have been checking with breakpoints, that is how I added the other query. Let me take a look, maybe I did not commit something.

[ekaterinadimitrova2]

Oh I missed the second comment, thanks for adding that. I will add it to the test, not sure what happened that I missed it.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[cassci-bot]

✔️ Build ds-cassandra-pr-gate/PR-2115 approved by Butler

---

Approved by Butler
See build details here