8000 [Feature] Change bucket number from physical partition level to materialized index level by xiangguangyxg · Pull Request #59441 · StarRocks/starrocks · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Feature] Change bucket number from physical partition level to materialized index level #59441

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

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

xiangguangyxg
Copy link
Contributor
@xiangguangyxg xiangguangyxg commented May 28, 2025

Why I'm doing:

This is a preliminary work of tablet splitting and merging.
Previously, bucket number is at physical partition level. All materialized indexes in a physical partition must have the same bucket number. But after tablet splitting, different materialized index in a physical partition could have different bucket number. We need to change bucket number from physical partition level to materialized index level.

What I'm doing:

Change bucket number from physical partition level to materialized index level.

Because different materialized index in a physical partition could have different bucket number. Tablet sink cannot calculate a unified tablet index for each record of data to be distributed to different materialized index.
To solve the problem, we refactor the code of tablet sink. Now tablet sink calculate a unified hash value for each record of data to be distributed to different materialized index, then the tablet index for each record of data will be calculated according to the hash value and the tablet size of a materialized index when the record of data is distributed to the materialized index.

This pr just refactor the code. Next pr will remove num_bucket in OlapTablePartition and use tablets.size() in each OlapTableIndexTablets instead.

Fixes #59134

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.5
    • 3.4
    • 3.3
    • 3.2
    • 3.1

Copy link
Contributor
@alvin-celerdata alvin-celerdata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can finish the necessary refactoring work in this PR, postpone the behavior change and protocol change in the future PR.
So you can just keep not changing the num_bucket, just refactoring the tabel_reader and tablet_sink in the backend.

@@ -258,7 +257,7 @@ class OlapTablePartitionParam {
// `invalid_row_index` stores index that chunk[index]
// has been filtered out for not being able to find tablet.
// it could be any row, becauset it's just for outputing error message for user to diagnose.
Status find_tablets(Chunk* chunk, std::vector<OlapTablePartition*>* partitions, std::vector<uint32_t>* indexes,
Status find_tablets(Chunk* chunk, std::vector<uint32_t>* hashes, std::vector<OlapTablePartition*>* partitions,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest not changing the order of the parameters in this PR. It introduces an extra burden to track what you are doing.
To keep it a renaming change makes life easier.

Copy link
Contributor Author
@xiangguangyxg xiangguangyxg May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I update the code to keep the order of the parameters in the function find_tablets.
But I still refactor the parameters in the private function _find_tablets_with_range_partition and _find_tablets_with_list_partition. Because their original parameters cannot tell which are input and which are output, and even the original function comments are written incorrectly, some input parameters are incorrectly annotated as output parameters.

Copy link

Copy link
Contributor
@alvin-celerdata alvin-celerdata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…ialized index level

Signed-off-by: xiangguangyxg <xiangguangyxg@gmail.com>
Copy link

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

[FE Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

[BE Incremental Coverage Report]

fail : 18 / 41 (43.90%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 be/src/exec/tablet_sink_colocate_sender.cpp 0 9 00.00% [43, 62, 63, 64, 66, 77, 78, 79, 81]
🔵 be/src/exec/tablet_info.h 0 1 00.00% [174]
🔵 be/src/exec/tablet_info.cpp 6 13 46.15% [571, 573, 589, 626, 637, 696, 699]
🔵 be/src/exec/tablet_sink.cpp 2 4 50.00% [680, 702]
🔵 be/src/exec/tablet_sink_sender.cpp 4 8 50.00% [61, 62, 63, 65]
🔵 be/src/storage/table_reader.cpp 6 6 100.00% []

@alvin-celerdata alvin-celerdata merged commit 4b9f9e2 into StarRocks:main Jun 3, 2025
57 of 58 checks passed
@xiangguangyxg xiangguangyxg deleted the bucket_num branch June 4, 2025 01:18
AntiTopQuark pushed a commit to AntiTopQuark/starrocks that referenced this pull request Jun 19, 2025
…ialized index level (StarRocks#59441)

Why I'm doing:
This is a preliminary work of tablet splitting and merging.
Previously, bucket number is at physical partition level. All materialized indexes in a physical partition must have the same bucket number. But after tablet splitting, different materialized index in a physical partition could have different bucket number. We need to change bucket number from physical partition level to materialized index level.

What I'm doing:
Change bucket number from physical partition level to materialized index level.

Because different materialized index in a physical partition could have different bucket number. Tablet sink cannot calculate a unified tablet index for each record of data to be distributed to different materialized index.
To solve the problem, we refactor the code of tablet sink. Now tablet sink calculate a unified hash value for each record of data to be distributed to different materialized index, then the tablet index for each record of data will be calculated according to the hash value and the tablet size of a materialized index when the record of data is distributed to the materialized index.

This pr just refactor the code. Next pr will remove num_bucket in OlapTablePartition and use tablets.size() in each OlapTableIndexTablets instead.

Fixes StarRocks#59134

Signed-off-by: xiangguangyxg <xiangguangyxg@gmail.com>
Signed-off-by: AntiTopQuark <AntiTopQuark1350@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic tablet splitting and merging
4 participants
0