[improve][metadata] Add partition key resolver support for Oxia index scans#26148
[improve][metadata] Add partition key resolver support for Oxia index scans#26148void-ptr974 wants to merge 2 commits into
Conversation
| Set<Option> getOpts = OptionsHelper.withResolvedPartitionKey(opts, key); | ||
| chain = chain | ||
| .thenCompose(__ -> storeGet(key, opts)) | ||
| .thenCompose(__ -> storeGet(key, getOpts)) |
There was a problem hiding this comment.
The resolver is evaluated eagerly during the chain assembly. Because the first thenCompose is attached to an already-completed future, an earlier storeGet may already be in flight when a later resolver throws. The outer stage then calls onError, but the detached get can still complete and invoke onNext afterward, which violates scanByIndex’s terminal-callback contract. Resolve the partition key inside the sequential thenCompose stage and add a resolver-failure ordering test.
There was a problem hiding this comment.
Fixed. The partition-key resolution now happens inside the sequential thenCompose, so the resolver for the next primary key is not evaluated until the preceding storeGet has completed.
Added a regression test that holds the first read pending and makes the second resolver fail. It verifies that the scan remains pending before the first read completes, then observes onNext followed by onError, with no read for the second key.
| } | ||
| for (Option o : opts) { | ||
| if (o instanceof Option.PartitionKeyResolver resolver) { | ||
| String partitionKey = resolver.resolver().apply(path); |
There was a problem hiding this comment.
The PartitionKeyResolver explicitly allows returning null, but the current behavior then performs a lookup without a partition key. If the indexed record was originally stored with a custom partition key, this lookup can access a different shard and return an empty result—silently dropping the indexed data while the scan reports success. Could we either cause the scan to fail when routing cannot be resolved or, alternatively, clearly document and test the precise conditions under which the unpartitioned fallback is safe?
There was a problem hiding this comment.
Fixed. PartitionKeyResolver now requires a non-null key for every scanned primary path. Returning null fails the scan with an IllegalStateException instead of falling back to an unpartitioned read, preventing a silent cross-shard miss.
Added a test that verifies the error callback and confirms that no primary read is issued when the resolver cannot provide a key.
| String parent = "/partition-key-index-scan-" + System.nanoTime(); | ||
| String indexName = "idx:partition-key-resolver-" + System.nanoTime(); | ||
| String indexKey = "match"; | ||
| RoutedKey routedKey = createIndexedKeyOnlyVisibleWithPartitionKey(store, parent, indexName, indexKey); |
There was a problem hiding this comment.
This helper may try multiple candidate records, but every unsuccessful candidate is deleted and it returns immediately after finding the first cross-shard record. Therefore only one indexed record exists when scanByIndex runs. The motivating case is a single scan containing primary records with different partition keys. Could the test retain at least two cross-shard records with distinct partition keys and assert that both are returned? This would catch an implementation that accidentally resolves or caches one partition key per scan rather than per result.
There was a problem hiding this comment.
Updated. The integration test now retains two cross-shard indexed records with distinct partition keys and asserts that both primary paths are returned by the scan. This exercises per-result resolution rather than a single scan-level routing key.
The focused unit and multi-shard Oxia integration tests pass.
Motivation
Oxia records can be routed with
PartitionKey. Native secondary-index scans return primary paths, and some callers can derive the routing key from each returned path. A single scan-levelPartitionKeyonly covers scans scoped to one routing key; it does not cover scans that may span multiple routing keys.For example, transaction op records are stored under
/txn/op/<txnId>-<seq>withPartitionKey(txnId). A scan by segment can return op records for multiple transaction IDs, so each follow-up primary read can use the transaction ID derived from the returned path.This PR adds an option for passing that per-result routing hint and uses it in transaction metadata index scans.
Modifications
Option.PartitionKeyResolverto derive a concretePartitionKeyfrom a primary path discovered during a scan.PartitionKeybefore Oxia loads each primary record returned byscanByIndex.Verifying this change
This change added tests and can be verified as follows:
./gradlew :pulsar-metadata:compileTestJava :pulsar-broker:compileJava :pulsar-metadata:checkstyleMain :pulsar-metadata:checkstyleTest :pulsar-broker:checkstyleMain -x :managed-ledger:compileTestJava./gradlew :pulsar-metadata:test --tests org.apache.pulsar.metadata.OxiaPartitionKeyTestDoes this pull request potentially affect one of the following parts:
This adds a new metadata-store
Optionsubtype. It does not change existing option behavior, configuration defaults, wire protocol, or metadata format.