Allow custom SAI components discovery #1338
Open
+1,428
−318
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pcmanus
force-pushed
the
cndb-10889-customizable-index-discovery
branch
5 times, most recently
from
e19c101
to
dacb203
Compare
pcmanus
force-pushed
the
cndb-10889-customizable-index-discovery
branch
11 times, most recently
from
32f9a1b
to
362bf10
Compare
pcmanus
force-pushed
the
cndb-10889-customizable-index-discovery
branch
from
6f1346d
to
2961010
Compare
eolivelli
reviewed
Oct 29, 2024
| private volatile boolean isComplete; | ||
| // Mark groups that are being read/have been fully written, and thus should not have new components added. | ||
| // This is just to catch errors where we'd try to add a component after the completion marker was written. | ||
| private volatile boolean sealed; |
There was a problem hiding this comment.
is "sealed" a reserved keyword in some newer java version ?
There was a problem hiding this comment.
Yes and no. It's a "contextual reserved keyword" in java 16+ but this is not a context where it is reserved (it's only in class/interface names).
pcmanus
force-pushed
the
cndb-10889-customizable-index-discovery
branch
2 times, most recently
from
9b22677
to
16b0adc
Compare
jasonstack
approved these changes
Nov 4, 2024
| boolean isUsable = candidate.expectedComponentsForVersion().stream().noneMatch(candidate::componentExistsOnDisk); | ||
| if (!isUsable) | ||
| { | ||
| noSpamLogger.warn(logMessage("Wanted to use generation {} for new build of {} SAI components of {}, but found some existing components on disk for that generation (maybe leftover from an incomplete/corrupted build?); trying next generation"), |
There was a problem hiding this comment.
nit: would be nice to include sstable id in the log
There was a problem hiding this comment.
Unless I misunderstand what you mean by "sstable id", I believe it's in there already: the last arg of the message is descriptor which is the sstable Descriptor and thus includes the id.
src/java/org/apache/cassandra/index/sai/disk/format/IndexDescriptor.java
Outdated
Show resolved
Hide resolved
Refactor the code that "discover" which SAI components files an SSTable has to allow making that code customizable. The default behavior remains the existing one, that is the TOC file is consulted, with a fallback to scanning disk if the TOC file is missing or corrupted. The way to customize this behavior is similar to what `SSTableWatcher` does: a new system property, `-Dcassandra.sai.custom_components_discovery_class` allows to load a specific class (that extends the `IndexCoomponentDiscovery` class) instead of the default. As part of the refactor to make this new customisability easier, this introduces a new `ComponentsBuildId` class that is the pair of the version and generation of a group of components, since this is what ultimately defines a given component "group" build.
It's easy to do so, and easier for implementation of that callback if they need it (they could get it indirectly from the sstable descriptor keyspace and table name, but passing it is easier).
pcmanus
force-pushed
the
cndb-10889-customizable-index-discovery
branch
from
16b0adc
to
e3a70a8
Compare
|
❌ Build ds-cassandra-pr-gate/PR-1338 rejected by Butler1 new test failure(s) in 4 builds Found 1 new test failures
Found 14 known test failures |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the code that "discover" which SAI components files an SSTable has to allow making that code customizable. The default behavior remains the existing one, that is the TOC file is consulted, with a fallback to scanning disk if the TOC file is missing or corrupted.
The way to customize this behavior is similar to what
SSTableWatcherdoes: a new system property,-Dcassandra.sai.custom_components_discovery_classallows to load a specific class (that extends theIndexCoomponentDiscoveryclass) instead of the default.As part of the refactor to make this new customisability easier, this introduces a new
ComponentsBuildIdclass that is the pair of the version and generation of a group of components, since this is whatultimately defines a given component "group" build.This PR is used by https://github.com/riptano/cndb/pull/11361 on the CNDB side, in the context of https://github.com/riptano/cndb/issues/10889.