fix(queryplanner): fix metadata query routing to multiple partitions - #2130
fix(queryplanner): fix metadata query routing to multiple partitions#2130bhavanap1131-cmd wants to merge 7 commits into
Conversation
amolnayak311
left a comment
There was a problem hiding this comment.
Please follow the connections given in https://github.com/filodb/FiloDB/blob/develop/CONTRIBUTING.md and update the PR title, and provide the summary of the changes
|
@bhavanap1131-cmd Please confirm all review comments are adequately addressed before requesting further reviews. |
alextheimer
left a comment
There was a problem hiding this comment.
Huge thanks for all the iteration @bhavanap1131-cmd
This PR can be merged after this one last comment is addressed.
- Add resolveMetadataPartitions with fallback to getMetadataPartitions when shard key filters are non-Equals or useLegacyMetadataRouting is set - Pass TaskWorkUnit to PartitionAssignment - Add tests for fallback routing in MultiPartitionPlannerSpec - Add useLegacyMetadataRouting config to QueryConfig/RoutingConfig Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
05cb723 to
d58ebc5
Compare
alextheimer
left a comment
There was a problem hiding this comment.
It looks like some tests are still missing. What about should not throw for regex filters on shard keys in metadata queries? That is mentioned in a comment just prior to the force-push.
Couple other tests that look like they are missing (just a few I see in the PR comments):
- should use getPartitions directly when a shard-key filter is missing
- should fallback to getMetadataPartitions when one shard-key filter is regex
- should handle non-equals filters gracefully in getPartitions delegation
There might be others, too. I would recommend using git reflog to find the state of the repo immediately prior to this commit. You can open a separate PR with that state, and I'll take a look. If that looks good, we can hard-reset the state of the original PR to that branch.
Pull Request checklist
Current behavior:
Metadata queries (LabelValues, SeriesKeysByFilters, LabelNames, etc.) in MultiPartitionPlanner were
unconditionally routed through getMetadataPartitions (legacy fallback path), regardless of whether the
shard-key filters were sufficient for direct partition routing. SeriesKeysByFilters plans were also not
handled correctly since their filters are stored directly on the plan node rather than on RawSeries
leaves.
New behavior:
Introduced resolveMetadataPartitions which routes directly via getPartitions when all shard-key filters
are present and Equals, falling back to getMetadataPartitions otherwise. SeriesKeysByFilters plans are
now handled explicitly. Test coverage added in MultiPartitionPlannerSpec for all routing conditions
across both plan types. Broken and misplaced tests removed from PlannerHierarchySpec.
Other information:
processMultiPartition = true -- the partition provider was never called. Behaviour is covered in
MultiPartitionPlannerSpec.