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

fix(query): ShardKeyRegexPlanner batching fix #1716

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

alextheimer
Copy link
Contributor

Pull Request checklist

  • The commit(s) message(s) follows the contribution guidelines ?
  • Tests for the changes have been added (for bug fixes / features) ?
  • Docs have been added / updated (for bug fixes / features) ?

Plans are materialized directly by inner planners when they draw data from one partition. However, when the ShardKeyRegexPlanner fanout batch size is small enough, multiple plans may be produced for one partition. Currently, these plans are concatenated and returned; they will be missing any root-level e.g. aggregations/joins.

This PR ignores the "if one partition then materialize with inner planners" logic when the inner planners return more than one plan.

For example, plans prior to this PR may be produced as:

Concat
    Agg
        RawData(localShardKey1)
        RawData(localShardKey1)
    Agg
        RawData(localShardKey2)
        RawData(localShardKey2)

Now, root plans will be correct because materialization proceeds through the ShardKeyRegexPlanner's walkLogicalPlanTree when inner planners produce more than one plan:

Agg
    Agg
        RawData(localShardKey1)
        RawData(localShardKey1)
    Agg
        RawData(localShardKey2)
        RawData(localShardKey2)

Plans are materialized directly by inner planners when they draw data from one partition.
However, when the ShardKeyRegexPlanner fanout batch size is small enough, multiple plans
may be produced for one partition. Higher-level coordination is then needed for e.g.
aggregations. This commit re-materializes plans with higher-level coordination from
the ShardKeyRegexPlanner when batching produces more than one plan.
@alextheimer alextheimer merged commit aecfbeb into filodb:develop Feb 15, 2024
1 check passed
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.

2 participants