-
Notifications
You must be signed in to change notification settings - Fork 20
CNDB-15805: Log the reason SAI index build was stopped #2196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Checklist before you submit for review
|
|
Code coverage shows 25% coverage and it shows that I haven't covered the log lines changes. This is inaccurate. I checked with breakpoints both the changed and new tests in Then if I go to cloud bees - I see for some of the unit test jobs that there was issue collecting all logs. It doesn't say what exactly, but I guess this may have been the problem. I found in the logs: [junit-timeout] Test org.apache.cassandra.index.sai.cql.NativeIndexDDLTest FAILED It seems this was the first test class running in that split and the rest of the test classes were not running at all. I also noticed a note about Jenkins agent being offline. So I suspect this run was interrupted or something. I will rebase and re-trigger CI. |
…paused" This reverts commit 83e43c1.
e7a4134 to
64047c7
Compare
|
❌ Build ds-cassandra-pr-gate/PR-2196 rejected by Butler3 regressions found Found 3 new test failures
Found 3 known test failures |
|
concurrentTruncateWithIndexBuilding failed in the last run, but I think it is just a flaky test. My suggestion - we remove checking the particular log as an overkill or we extend the |



What is the issue
...
When SAI catches
CompactionInterruptedException, we only log that index build was stopped, but we don't log the most important information contained in the CompactionInterruptedException. And it stores e.g. the reason of it being thrown.What does this PR fix and why was it fixed
...
Log the reason SAI index build was stopped.
Testing showed a gap in our testing. The actual gap turned out to be actually a broken test that said it is testing what something it was not actually testing. I fixed it as part of this PR and added additional test too.
So this PR covers both - #15805 and #16415. I actually suggest we close #16415 in favor of #15805.