Skip to content
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

CNDB-12154: Port latest commits from main to main-5.0 #1467

Open
wants to merge 139 commits into
base: main-5.0
Choose a base branch
from

Conversation

djatnieks
Copy link

@djatnieks djatnieks commented Dec 18, 2024

What is the issue

The main-5.0 branch needs updating with the latest commits from main

What does this PR fix and why was it fixed

Ports the latest commits from main to main-5.0.

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

michaeljmarshall and others added 30 commits November 21, 2024 12:44
This additional metadata is somewhat valuable in the context of troubleshooting. Recently, we had an issue where the checksum itself was not (over)written and so it was stored as 0. In many cases, this won't be helpful, but since it is cheap and could be helpful, I propose adding some additional metadata when checksums don't match.
* Implement FSError#getMessage to ensure file name is logged

For this code block:

```java
var t = new FSWriteError(new IOException("Test failure"), new File("my", "file"));
logger.error("error", t);
```

We used to log:

```
ERROR [main] 2024-09-19 11:09:18,599 VectorTypeTest.java:118 - error
org.apache.cassandra.io.FSWriteError: java.io.IOException: Test failure
	at org.apache.cassandra.index.sai.cql.VectorTypeTest.endToEndTest(VectorTypeTest.java:117)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:232)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)
Caused by: java.io.IOException: Test failure
	... 42 common frames omitted
```

Now we will log:

```
ERROR [main] 2024-09-19 11:10:02,910 VectorTypeTest.java:118 - error
org.apache.cassandra.io.FSWriteError: my/file
	at org.apache.cassandra.index.sai.cql.VectorTypeTest.endToEndTest(VectorTypeTest.java:117)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:232)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)
Caused by: java.io.IOException: Test failure
	... 42 common frames omitted
```

* Add super.getMessage to message
* Use query view's locked indexes for Plan#estimateAnnNodesVisited

This commit doesn't resolve the underlying problem
in the design: we could easily use the wrong
reference at any time. I'll need to think
on this a bit more to know what is best.

* Assert queryView is not null
This commit fixes a serious correctness bug in the way we build
RowFilter for expressions involving OR.

If a query contained multiple complex predicates such as NOT IN
joined with the OR operator, the slices produced by NOT IN
were incorrectly also joined by OR instead of by AND.

In addition, a NOT IN with an empty list, if ORed with another
expression, was incorrectly treated as an expression matching 0 rows,
instead of matching all rows.

Example 1:
SELECT * FROM t WHERE x = 1 OR x NOT IN (2, 3, 4)
was incorrectly matching all rows, including the ones
with x = 2 or x = 3 or x = 4.

Example 2:
SELECT * FROM t WHERE x = 1 OR x NOT IN ()
was incorrectly matching only row with x = 1, instead of all rows.

The bug was technically not limited to NOT IN, but any
single restriction that wanted to add in exactly zero or more than one
filter expression.

Fixes riptano/cndb#10923
Fix typo in the comment

Co-authored-by: Andrés de la Peña <adelapena@users.noreply.github.com>
Javadoc

Co-authored-by: Andrés de la Peña <adelapena@users.noreply.github.com>
…ted (#1238)" (#1301)

This commit does not work because the queryView is not set until
after we get the plan. Since the view is only used to estimate
the cost of work and not to skip work, it is safe to use a
different view.

This reverts commit 9333708.
…n overridable method… (#1296)

This commit simply move the code that deletes index components locally
when an index is dropped inside a new method of the `SSTableWatcher`
interface. As is, this just creates a minor indirection without changing
any behaviour, but this allows custom implementations of
`SSTableWatcher` to modify this behaviour, typically to handle tiered
storage concerns.
* Added unified_compaction.override_ucs_config_for_vector_tables option.  When enabled, the Controller will use the preferred vector settings for vector tables.
- Added additional options for vector specific configuration.
…hards (#1255)

Implement `ShardManagerReplicaAware` to align UCS and replica shards and thus limit the amount of sstables that are partially owned by replicas.

The most interesting details are in the `IsolatedTokenAllocator#allocateTokens` and the `ShardManagerReplicaAware#computeBoundaries` methods.

In the `allocateTokens` method, we take the current token metadata for a cluster, replace the snitch with one that does not gossip, and allocate new nodes until we satisfy the desired `additionalSplits` needed. By using the token allocation algorithm, high level split points naturally align with replica shards as new nodes are added.

In `computeBoundaries`, we allocate any tokens needed, then we split the space into even spans and find the nearest replica token boundaries.
CNDB-10988: inspect out-of-space exception on compaction
- count evictions and not bytes
- add metrics by removal cause
…nts of a collection and/or UDT

Port of DB-1289/CASSANDRA-8877
)

CNDB-10945: Change calculation of sstable span for small sstables

In addition to correcting for very small spans, this also corrects sstable
spans for ones having a small number of partitions where keys can easily
fall in a small range. For these cases we use a value derived from the
number of partitions in the table, which should work well for all
scenarios, including wide-partition tables with a very limited number of
partitions.
The eagerly populated leafNodeToLeafFP TreeMap has been replaced
with a LeafCursor which allows to traverse the index tree directly,
in a lazy way.

The change significantly reduces the amount of up-front work we did
to initialize the BKDReader.IteratorState.
It also reduces GC pressure and memory usage.

The user-facing effect is that `ORDER BY ... LIMIT ...`
queries using a numeric index (KD-tree) are significantly faster.

Fixes riptano/cndb#11021
Implements Stage 2 of Trie memtables, implementing trie-backed partitions
and extending the memtable map to the level of tries. This stage still
handles deletions with the legacy mechanisms (RangeTombstoneList etc)
but can save quite a bit of space for the B-Tree partition-to-row maps.

Also includes:
- Code alignment with the Cassandra 5.0 branch.
- A port of the OSS50 byte-comparable encoding version.
- Fixed version preencoded byte-comparables for version safety.
- Duplication of byte sources and better toArray conversion.
- Direct skipping mechanism for trie cursors and map-like get method.
- Forced node copying mechanism for trie mutations for atomicity and consistency.
- Pluggable cell reuse.
- Prefix and tail tries, as well as filtered iteration.
- A mechanism for extracting current key during trie mutation.
- Volatile reads to fully enforce happens-before between node preparation and use.
- Various in-memory trie improvements.

The earlier trie memtable implementation is still available as TrieMemtableStage1.
* DefaultMemtableFactory: align entire implementation with main branch
* TrieMemtable: restore FACTORY instance var and factory(Map) method
* TrieMemoryIndex: add previously missed use of BYTE_COMPARABLE_VERSION in rangeMatch method
…gregate and use picked sstables size as maxOverlap for aggregate (#1309)

CNDB-10990: include archive size in Level.avg and use maxOverlap for unified aggregate

update getOverheadSizeInBytes to include non-data components size

Add config to disable new behavior, default enabled. add tests
…aybe we still want to check newer version for JDK22 specifically.

Though this is the last ecj version to support JDK11.
Upgrade:
- ecj plus fix the java udf functions for JDK11+
- snakeyaml - it was already bumped in CNDB for security vulnerability
- test dependencies:
   jacoco, byteman - higher version than CNDB but it is needed for catching up on JDK22 in tests
   findbugs - aligned with CNDB version but we probably want at some point to get to major version upgrade; not a priority for now
   jmh, bytebuddy - bumped to latest versions as they are known for not working on newer JDK versions
This commit improves performance of appending SAI components
by avoiding unnecessary computation of CRC from the beginning
of the file each time it is opened for appending.

Fixes riptano/cndb#10783
@djatnieks djatnieks marked this pull request as ready for review December 19, 2024 23:53
@djatnieks djatnieks changed the title CNDB-12154 CNDB-12154: Port latest commits from main to main-5.0 Dec 19, 2024
djatnieks and others added 10 commits December 23, 2024 10:51
…g VIntOutOfRangeException to the catch block of SSTableIdentityIterator.exhaust method.
…pactionProgress to return the operation type from the first entry in the shared progress; needed in cases that a CompactionTask type is changed after creation.
…opriate (#1469)

Fixes riptano/cndb#12239

We found the System.nanoTime was using significant cpu cost, but because
the timeout is high enough, we can accept the inaccuracy.

- [ ] 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
Fixes regression in jvector 3.0.4 when compacting PQVectors larger than 2GB
### What is the issue
SimpleClientPerfTest has been failing in CI since changes from
CNDB-10759

### What does this PR fix and why was it fixed
This change in `SimpleClientPerfTest`, updates the anonymous class
`Message.Codec<QueryMessage>` to override the correct method, `public
CompletableFuture<Response> maybeExecuteAsync` from `QueryMessage`,
whose signature was changed as part of CNDB-10759.

### 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
…ing for async batchlog removal (#1485)

The test asserts that the batchlog is removed immediately after the
write completes, but removal of the batchlog is async and can be
delayed, particularly in resource-limited environments like CI.
The test generates partition index accesses by reusing the same key, and if the key cache is enabled, the test will fail for bigtable profiles because the key will be in the key cache.
…by filtering queries (#1484)

Queries creating fake index contexts each create their own context,
which can then race on metric registration (as the metrics have the same
patterns). This can cause a query to fail. These metrics are superfluous, 
we can skip creating them entirely.
/**
* This class exists solely to avoid initialization of the default memtable class.
* Some tests want to setup table parameters before initializing DatabaseDescriptor -- this allows them to do so.
*/
public class DefaultMemtableFactory
public class DefaultMemtableFactory implements Memtable.Factory
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems I forgot in CNDB-9850 that the OSS branch has a better solution to the problem this is meant to solve: extract the factory outside the memtable class, so that it no longer has a dependency on the memtable class being constructed. On the OSS branch that's the SkipListMemtableFactory, here we can do the same moving TrieMemtable.Factory to top level as TrieMemtableFactory, and then referencing its INSTANCE from the default params no longer involves initializing TrieMemtable.

This avoids having to reproduce every method of Memtable.Factory here which we can easily forget to do.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll do that

djatnieks and others added 9 commits January 6, 2025 18:55
…ate.accumulatedDataSize; it only worked to fix SensorsWriteTest.testMultipleRowsMutationWithClusteringKey for SkipListMemtable and may not be necessary if the default memtable becomes TrieMemtable. Revisit SensorsWriteTest later if necessary.
Move static class TrieMemtable.Factory to TrieMemtableFactory class;
Use suggested TriePartitionUpdate.unsharedHeapSize implementation;
Use InMemoryTrie.shortLived in TrieToDotTest and TrieToMermaidTest;
Add specific versions aa, ca, and da to RowIndexTest;
Add addMemoryUsageTo in SkipListMemtable and TrieMemtable
Add TrieMemtable.switchOut
Enqueue start time will now be correctly measured for every API for task
execution

Enqueue start times are measured at a wrong moment for some APIs.

We now measure the enqueue start time as a time of entry to each task
execution API.
It was fixed so that we see the correct enqueue times in metrics
… while updating metrics (#1482)

a JVM system property is read and parsed on the hotpath

Cache the value on a constant

Co-authored-by: Joel Knighton <joel.knighton@datastax.com>
#1295)

Addresses: riptano/cndb#8501
CNDB PR: riptano/cndb#11681

The idea is to expose sensors files to CQL client via the native
protocol's custom payload (which is available in V4 or higher). This
first iteration add `READ_BYTES` and `WRITE_BYTES`.
jkni and others added 4 commits January 14, 2025 19:01
…for tidiers to run (#1498)

This test relies on file system state being blank, so truncating isn't enough unless we wait for tidiers to run.
QueryView#build fell into an infinite loop in January certification which led to query timeouts.

This PR fundamentally changes the algorithm used in QueryView#build.

Instead of matching sstables and memtables to indexes we know, we go in the opposite direction now: we find a corresponding index for each sstable and memtable. That simplifies the code and reduces the need for retrying.

Now we only need to retry the memtable index lookup failures because memtables are not refcounted
explicitly like sstables so we can't prevent their concurrent release. But even retrying after a memtable index lookup failure is limited to once per memtable now.

We don't have to retry sstable index lookups, because as long as we hold the refViewFragment with the referenced sstables, their indexes won't be released, so the lookups should never fail.
…eComparable (#1504)

### What is the issue
Fixes: riptano/cndb#12445

### What does this PR fix and why was it fixed
Appears to be a regression introduced by 338902c or #1177

The general issue is that SAI uses `OSS41` to encode its primary keys in the trie index, but this code path attempted to interpret the clustering columns using `OSS50`.
@cassci-bot
Copy link

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


63 new test failure(s) in 12 builds
See build details here


Found 63 new test failures

Showing only first 15 new test failures

Test Explanation Branch history Upstream history
o.a.c.d.c.CompactionControllerTest.testMemtable... regression 🔴🔴🔴🔴🔴🔴🔴
...adCommitLogAndSSTablesWithDroppedColumnTestCC40 regression 🔴🔴🔴🔴🔴🔴🔴
...adCommitLogAndSSTablesWithDroppedColumnTestCC50 regression 🔴🔴🔴🔴🔴🔴🔴
...ersionTest.v4ConnectionCleansUpThreadLocalState regression 🔴🔴🔴🔴🔴🔴🔴
...rtitionRestrictedQueryTest.testSAIFailThreshold regression 🔴🔴🔴🔴🔴🔴🔴
...rtitionRestrictedQueryTest.testSAIWarnThreshold regression 🔴🔴🔴🔴🔴🔴🔴
...eadSizeWarningTest.warnThresholdSinglePartition regression 🔴🔴🔴🔴🔴🔴🔴
...lIndexImplementationsTest.testDisjunction[SASI] regression 🔴🔴🔴🔴🔴🔴🔴
o.a.c.i.c.CompressionMetadataTest.testMemoryIsF... regression 🔴🔴🔴🔴🔴🔴🔴
....i.c.CompressionMetadataTest.testMemoryIsShared regression 🔴🔴🔴🔴🔴🔴🔴
...ientRequestMetricsLatenciesTest.testReadMetrics regression 🔴🔴🔴🔴🔴🔴🔴
...entRequestMetricsLatenciesTest.testWriteMetrics regression 🔴🔴🔴🔴🔴🔴🔴
...ntIrWithPreviewFuzzTest.concurrentIrWithPreview regression 🔴🔴🔴🔴🔴🔴🔴
o.a.c.t.SSTablePartitionsTest.testMinRows regression 🔴🔴🔴🔴🔴🔴🔴
o.a.c.t.TransportTest.testAsyncTransport regression 🔴🔴🔴🔴🔴🔴🔴

Found 33 known test failures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.