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

[Enhancement] Materialized views support refresh granularity splits #12926

Merged
merged 11 commits into from Nov 8, 2022

Conversation

Astralidea
Copy link
Contributor

What type of PR is this:

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

Which issues of this PR fixes :

Fixes #12925

Problem Summary(Required) :

Checklist:

  • I have added test cases for my bug fix or my new feature
  • I have added user document for my new feature or new function

@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

throw new AnalysisException("Partition Refresh Number: " + e.getMessage());
}
if (partitionRefreshNumber <= 0) {
throw new AnalysisException("Partition Refresh Number should larger than 0.");
Copy link
Contributor

Choose a reason for hiding this comment

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

how to unset refresh number?
what if the user do not specify the refresh number, what is the default logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feature will do in Support alter materialized view properties or later PR

}

@VisibleForTesting
public void filterPartitionByRefreshNumber(Set<String> partitionsToRefresh, MaterializedView materializedView) {
Copy link
Contributor

Choose a reason for hiding this comment

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

have you test if one source base table partition has two target mv partitions can run successfully?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, this configuration has nothing to do with the source table, only the number of partitions of the materialized view table.

gengjun-git
gengjun-git previously approved these changes Nov 7, 2022
@Astralidea Astralidea enabled auto-merge (squash) November 7, 2022 07:28
Youngwb
Youngwb previously approved these changes Nov 7, 2022
@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

@Astralidea
Copy link
Contributor Author

run starrocks_fe_unittest

@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

@Astralidea
Copy link
Contributor Author

run starrocks_fe_unittest

@mofeiatwork
Copy link
Contributor

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Nov 8, 2022

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/6)
error: could not apply 2fecef310... Materialized views support refresh granularity splits
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 2fecef310... Materialized views support refresh granularity splits
Auto-merging fe/fe-core/src/main/java/com/starrocks/server/LocalMetastore.java
Auto-merging fe/fe-core/src/main/java/com/starrocks/scheduler/PartitionBasedMaterializedViewRefreshProcessor.java
Auto-merging fe/fe-core/src/main/java/com/starrocks/common/util/PropertyAnalyzer.java
Auto-merging fe/fe-core/src/main/java/com/starrocks/catalog/TableProperty.java
CONFLICT (content): Merge conflict in fe/fe-core/src/main/java/com/starrocks/catalog/TableProperty.java
Auto-merging fe/fe-core/src/main/java/com/starrocks/catalog/MaterializedView.java

err-code: AFE3D

@Astralidea
Copy link
Contributor Author

run starrocks_admit_test

@sonarcloud
Copy link

sonarcloud bot commented Nov 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@wanpengfei-git
Copy link
Collaborator

[FE PR Coverage Check]

😞 fail : 69 / 113 (61.06%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/alter/Alter.java 0 1 00.00% [277]
🔵 com/starrocks/common/util/PropertyAnalyzer.java 0 12 00.00% [202, 210, 218, 219, 221, 222, 223, 224, 225, 226, 228, 230]
🔵 com/starrocks/scheduler/ExecuteOption.java 0 1 00.00% [20]
🔵 com/starrocks/server/LocalMetastore.java 1 5 20.00% [3320, 3321, 3322, 3323]
🔵 com/starrocks/catalog/MaterializedView.java 3 6 50.00% [638, 639, 640]
🔵 com/starrocks/scheduler/PartitionBasedMaterializedViewRefreshProcessor.java 42 64 65.62% [175, 187, 227, 228, 229, 231, 236, 237, 238, 239, 240, 241, 242, 243, 245, 246, 247, 248, 250, 251, 252, 689]
🔵 com/starrocks/catalog/TableProperty.java 10 11 90.91% [145]
🔵 com/starrocks/qe/StmtExecutor.java 3 3 100.00% []
🔵 com/starrocks/scheduler/MvTaskRunContext.java 9 9 100.00% []
🔵 com/starrocks/clone/DynamicPartitionScheduler.java 1 1 100.00% []

@Astralidea Astralidea merged commit 7e59eca into StarRocks:main Nov 8, 2022
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.

Optimize the refresh logic of materialized views
6 participants