-
Notifications
You must be signed in to change notification settings - Fork 112
Allow index maintainer implementors to specify their own index match candidate #3640
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
Allow index maintainer implementors to specify their own index match candidate #3640
Conversation
…object instead of multiple parameters
…tead of a switch statement in MatchCandidate
...rc/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexMaintainerFactory.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexMaintainerFactory.java
Show resolved
Hide resolved
...rc/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexMaintainerFactory.java
Show resolved
Hide resolved
...in/java/com/apple/foundationdb/record/provider/foundationdb/IndexMaintainerRegistryImpl.java
Outdated
Show resolved
Hide resolved
* @return a collection of {@link MatchCandidate}s to match the index against | ||
*/ | ||
@Nonnull | ||
default Iterable<MatchCandidate> createMatchCandidates(@Nonnull RecordMetaData metaData, @Nonnull Index index, boolean reverse) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot remember the specifics but the reverse
here seems odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked it up and it's only the planning of RecordQuery
s that need this. Annoying...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I did notice that, but I didn't look too deeply into it. I'm not actually sure why we can't rely on the sort order on the sort descriptor. If we could, I'd be happy to remove this parameter
...er-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/CascadesPlanner.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/apple/foundationdb/record/query/plan/cascades/MatchCandidateExpansion.java
Show resolved
Hide resolved
* Moved the initiliazer of `CascadesPlanner` that took an `FDBRecordStoreBase` to a method on `FDBRecordStoreBase` * Expanded a comment that trailed off
This refactors the way that index match candidates are generated so that individual index maintainer factory implementations can override how this is done for the index types they manage. This includes plumbing through the store's
IndexMaintainerRegistry
to theCascadesPlanner
so that it can ask the right logic for the match candidate, as well as some refactoring done to theMatchCandidate
logic so that, if all goes well, index implementors can rely on library code as much as possible.As part of this approach, I modified the
IndexMaintainerRegistry
interface somewhat. That interface is backed by a map from index types toIndexMaintainerFactory
objects. The registry implementation is typically created once at JVM startup, using the service loader, though in theory someone could override that. This adds two new related interfaces,IndesMaintainerFactoryRegistry
, which returns access to the index maintainer factory directly, andIndexMatchCandidateRegistry
, which can create a match candidate for a given index. TheIndexMaintainerFactory
class is modified to have a new method to create match candidates, and theIndexMatchCandidateRegistry
will forward that request to the appropriateIndexMaintainerFactory
.This was done instead of putting a new method on the index maintainer itself. The idea there is to keep separate the concept of actually maintaining the index, which requires (for example) having a transaction opened, with the construction of artifacts that are related to the index. The index maintainer factory already had the ability to create both the index maintainer, as you might expect, but also the index validator. So this should be a natural extension to also create the index match candidate.
One other benefit: the index maintainer is bound to a single transaction. That would mean that if we wanted to separate out planning from the context of a transaction (e.g., offline planning), we'd have trouble if we were to do match candidate generation in the index maintainer itself. This aims to allow us to modify that in the future.
There's more refactoring of the
MatchCandidateInterface
which is designed to make it easy for use cases to create value-like match candidate definitions. There's a test class that shows that.