Add row count test for estimated row counts for SAI plan on single restrictions #1449

k-rus · 2024-12-03T18:14:03Z

Closes https://github.com/riptano/cndb/issues/12139

What is the issue

Changing planning can affect estimated row count of a plan. This test was introduced by #1439 to see how introducing anti-join plan node estimates row count.

What does this PR fix and why was it fixed

This PR adds a test of row count of a plan in the presence of restrictions. Currently it tests queries with inequality, equality and half-ranges on different SAI column types and with or without histograms.

Checklist before you submit for review

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

pkolaczk · 2024-12-04T13:45:52Z

test/unit/org/apache/cassandra/index/sai/plan/EstimatedRowCountTest.java

+public class EstimatedRowCountTest extends SAITester
+{


The test tests only the NEQ operator. It doesn't look like a general test for all estimated row counts.
Consider renaming it to NEQRowCountTest or some other name wich mentions this is a NEQ test only.

The test class includes only one test function now, which tests only inequality. I prepared the test and class in a way, so more tests can be added inside the class instead of adding new classes. I think it's aligned with existing tests. Let me know if I miss anything.
I will add another simple test, so it's less confusing.

I added more tests and improved the class name. Let me know if it needs to be improved.

pkolaczk · 2024-12-04T13:55:35Z

test/unit/org/apache/cassandra/index/sai/plan/EstimatedRowCountTest.java

+        SAIUtil.setLatestVersion(version);
+
+        ColumnFamilyStore cfs = prepareTable(type);
+        double totalRows = 97.0;


Where does this number come from?
You are loading 100 rows into the table, not 97.

But the table total count is 97 when I debug the code. I will check if I can retrieve the total count.

This estimated total is used in the plans factory. I changed to obtain it from the query controller.

@pkolaczk this number is calculated by dividing the size of a memtable by the row size. Actually it's probabilistic.

test/unit/org/apache/cassandra/index/sai/plan/EstimatedRowCountTest.java

k-rus · 2024-12-12T14:42:52Z

The test is flaky, see this build failure.
By enabling compression I was able to get different failures locally.
It seems that row count estimation is probabilistic. For memtable (and current test implementation uses only memtable) the total row count is calculated by dividing the size of the memtable by the row size.

k-rus · 2024-12-13T11:07:18Z

One local failure example:

Testcase: testHalfRangeMiddle(org.apache.cassandra.index.sai.plan.SingleRestrictionEstimatedRowCountTest):	FAILED
[junit-timeout] expected:<48.0> but was:<63.0>
[junit-timeout] junit.framework.AssertionFailedError: expected:<48.0> but was:<63.0>
[junit-timeout] 	at org.apache.cassandra.index.sai.plan.SingleRestrictionEstimatedRowCountTest$RowCountTest.doTest(SingleRestrictionEstimatedRowCountTest.java:121)
[junit-timeout] 	at org.apache.cassandra.index.sai.plan.SingleRestrictionEstimatedRowCountTest.testHalfRangeMiddle(SingleRestrictionEstimatedRowCountTest.java:145)
[junit-timeout] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit-timeout] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit-timeout] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit-timeout] 	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)

Another:

[junit-timeout] Testcase: testEquality(org.apache.cassandra.index.sai.plan.SingleRestrictionEstimatedRowCountTest):	FAILED
[junit-timeout] expected:<15.0> but was:<30.0>
[junit-timeout] junit.framework.AssertionFailedError: expected:<15.0> but was:<30.0>
[junit-timeout] 	at org.apache.cassandra.index.sai.plan.SingleRestrictionEstimatedRowCountTest$RowCountTest.doTest(SingleRestrictionEstimatedRowCountTest.java:121)
[junit-timeout] 	at org.apache.cassandra.index.sai.plan.SingleRestrictionEstimatedRowCountTest.testEquality(SingleRestrictionEstimatedRowCountTest.java:165)
[junit-timeout] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit-timeout] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit-timeout] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit-timeout] 	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)

k-rus · 2024-12-17T10:09:06Z

@pkolaczk I fixed the flakiness of the test by using a table builder and memtableFlushPeriod(0). Do you want to re-review the test?
My plan for extending the test for sstables is to use a separate PR.

Disables flush on the table as it affects the estimations being executed non-deterministically, resulting in race condition.

sonarqubecloud · 2024-12-19T10:36:20Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

cassci-bot · 2024-12-19T10:39:51Z

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

2 new test failure(s) in 8 builds
See build details here

Found 2 new test failures

Test	Explanation	Branch history	Upstream history
...adRepairGuarantees.test_atomic_writes[blocking]	regression	🔴🔴🔴🔵🔵🔵🔵	🔵🔵🔵🔵🔵🔵🔵
...,wide=false,scenario=POST_BUILD_QUERY]	regression	🔴🔵🔵🔵🔵🔵🔵	🔵🔵🔵🔵🔵🔵🔵

Found 19 known test failures

…strictions (#1449) Closes riptano/cndb#12139 This PR adds a test of row count of a SAI plan in the presence of restrictions. Currently it tests queries with inequality, equality and half-ranges on different SAI column types and with or without histograms.

k-rus self-assigned this Dec 3, 2024

k-rus requested a review from pkolaczk December 3, 2024 18:26

pkolaczk requested changes Dec 4, 2024

View reviewed changes

k-rus requested a review from pkolaczk December 5, 2024 19:19

k-rus force-pushed the rf-add-plan-row-count-test branch from 63c798f to 384df2c Compare December 10, 2024 11:08

pkolaczk approved these changes Dec 10, 2024

View reviewed changes

k-rus marked this pull request as draft December 12, 2024 14:28

k-rus force-pushed the rf-add-plan-row-count-test branch from 384df2c to 0cc54e3 Compare December 15, 2024 11:23

k-rus marked this pull request as ready for review December 17, 2024 10:07

k-rus changed the title ~~Add row count test for planned inequality~~ [DRAFT] Add row count test for planned inequality Dec 17, 2024

k-rus added 5 commits December 17, 2024 21:32

Add row count test for planned inequality

7bab891

Test more restrictions, thus refactor

162152c

Fix flakiness of row count estimate

180fc06

Disables flush on the table as it affects the estimations being executed non-deterministically, resulting in race condition.

Try control for no sstable is created

87eebf2

Avoid flushing by testing inside protected block

cba63d5

k-rus force-pushed the rf-add-plan-row-count-test branch from a3bc703 to cba63d5 Compare December 17, 2024 20:32

k-rus added 3 commits December 18, 2024 09:33

Try different scope for preventing flush

c98719e

Remove commented assertion code

4caf11a

Simplify the code and comment on the reasoning

4e547d7

k-rus changed the title ~~[DRAFT] Add row count test for planned inequality~~ Add row count test for estimated row counts for SAI plan on single restrictions Dec 19, 2024

k-rus merged commit 315a0e1 into main Dec 19, 2024
460 of 473 checks passed

k-rus deleted the rf-add-plan-row-count-test branch December 19, 2024 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add row count test for estimated row counts for SAI plan on single restrictions #1449

Add row count test for estimated row counts for SAI plan on single restrictions #1449

k-rus commented Dec 3, 2024 •

edited

Loading

pkolaczk Dec 4, 2024

k-rus Dec 4, 2024

k-rus Dec 5, 2024

pkolaczk Dec 4, 2024

k-rus Dec 5, 2024

k-rus Dec 5, 2024

k-rus Dec 12, 2024

k-rus commented Dec 12, 2024

k-rus commented Dec 13, 2024

k-rus commented Dec 17, 2024

sonarqubecloud bot commented Dec 19, 2024

cassci-bot commented Dec 19, 2024

Add row count test for estimated row counts for SAI plan on single restrictions #1449

Add row count test for estimated row counts for SAI plan on single restrictions #1449

Conversation

k-rus commented Dec 3, 2024 • edited Loading

What is the issue

What does this PR fix and why was it fixed

Checklist before you submit for review

pkolaczk Dec 4, 2024

Choose a reason for hiding this comment

k-rus Dec 4, 2024

Choose a reason for hiding this comment

k-rus Dec 5, 2024

Choose a reason for hiding this comment

pkolaczk Dec 4, 2024

Choose a reason for hiding this comment

k-rus Dec 5, 2024

Choose a reason for hiding this comment

k-rus Dec 5, 2024

Choose a reason for hiding this comment

k-rus Dec 12, 2024

Choose a reason for hiding this comment

k-rus commented Dec 12, 2024

k-rus commented Dec 13, 2024

k-rus commented Dec 17, 2024

sonarqubecloud bot commented Dec 19, 2024

Quality Gate passed

cassci-bot commented Dec 19, 2024

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

Found 2 new test failures

Found 19 known test failures

k-rus commented Dec 3, 2024 •

edited

Loading