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.

Sign up for GitHub

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

Jump to bottom

[Dynamic Partition] reserve specific history periods by dynamic partition. #6554

Merged
merged 16 commits into from
Sep 28, 2021
Merged

[Dynamic Partition] reserve specific history periods by dynamic partition. #6554

merged 16 commits into from
Sep 28, 2021

Conversation

Henry2SS
Copy link
Contributor

@Henry2SS Henry2SS commented Sep 2, 2021

Proposed changes

Add RESERVED_HISTORY_STARTS and RESERVED_HISTORY_ENDS.
Fixes #6514

Types of changes

What types of changes does your code introduce to Doris?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)
  • Code refactor (Modify the code structure, format the code, etc...)
  • Optimization. Including functional usability improvements and performance improvements.
  • Dependency. Such as changes related to third-party components.
  • Other.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • [ x] I have created an issue on (Fix #ISSUE) and described the bug/feature there in detail
  • Compiling and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • If these changes need document changes, I have updated the document
  • Any dependent changes have been merged

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@Henry2SS
Copy link
Contributor Author

Henry2SS commented Sep 2, 2021

I will add some unit-tests later.
And I have already compiled and validated this function in my own clusters.

@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch 4 times, most recently from f779054 to bda1918 Compare September 3, 2021 12:15
@morningman morningman added area/dynamic-partition kind/feature Categorizes issue or PR as related to a new feature. labels Sep 6, 2021
@morningman
Copy link
Contributor

hi @Henry2SS please update the document

@Henry2SS
Copy link
Contributor Author

Henry2SS commented Sep 6, 2021

hi @Henry2SS please update the document

Done.

@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from 88f8036 to d10cf87 Compare September 7, 2021 09:37
@Henry2SS Henry2SS changed the title modify dynamic partition. [Dynamic Partition] reserve specific history periods by dynamic partition. Sep 8, 2021
morningman
morningman reviewed Sep 8, 2021
View reviewed changes
docs/zh-CN/administrator-guide/dynamic-partition.md Outdated

```
["2020-06-01","2020-06-20"),
["2020-10-31","2020-11-15")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why using closed open range? Not closed range?

fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java Outdated
private static void checkReservedHistoryStarts(String reservedHistoryStarts) throws DdlException{
String[] starts = reservedHistoryStarts.split(",");
if (starts.length == 0) {
//ErrorReport.reportDdlException(ErrorCode.ERROR_DYNAMIC_PARTITION_RESERVED_HISTORY_STARTS_EMPTY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using ErrorReport.reportDdlException?

@Henry2SS
Copy link
Contributor Author

Henry2SS commented Sep 9, 2021

Hi, @morningman changed reserved history periods from closedOpen to closed. Thanks for your advice.

@caiconghui
Copy link
Contributor

how about comibine two configs into one config, use reserved_history_list instead of reserved_history_starts and reserved_history_ends, and we can set the property like "[2021-01-01, 2021-01-31] , [2021-02-03, 2021-02-15]", and use pattern to check the property and extract property

@Henry2SS
Copy link
Contributor Author

how about comibine two configs into one config, use reserved_history_list instead of reserved_history_starts and reserved_history_ends, and we can set the property like "[2021-01-01, 2021-01-31] , [2021-02-03, 2021-02-15]", and use pattern to check the property and extract property

Great idea. And i've finished changing it to reserved_history_periods. And ut will be modified later.

@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from a1605da to d12bccf Compare September 14, 2021 03:42
@caiconghui
Copy link
Contributor

please rebase master to solve the conflicts problem

@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from a0f623b to f5726d1 Compare September 14, 2021 04:08
@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch 3 times, most recently from d976679 to 8436d16 Compare September 14, 2021 05:15
@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from 8436d16 to 5a0d881 Compare September 14, 2021 05:25
@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from 864096b to 5331b4e Compare September 14, 2021 05:48
@Henry2SS
Copy link
Contributor Author

please rebase master to solve the conflicts problem

Done.

@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from 8bc295f to b605066 Compare September 16, 2021 09:49
@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from 8226393 to 91c0e3b Compare September 16, 2021 11:20
@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from c58a073 to 346cc33 Compare September 22, 2021 09:37
@Henry2SS Henry2SS mentioned this pull request Sep 22, 2021
Open
3 tasks
@Henry2SS Henry2SS requested a review from morningman September 23, 2021 12:38
@Henry2SS
Copy link
Contributor Author

@morningman Hi, it could reserve history periods when the TIME_UNIT is set to HOUR now. And modified docs and add two more unit-test to do validation.

@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch 2 times, most recently from 3f6d2db to 1233b3b Compare September 26, 2021 07:58
@Henry2SS Henry2SS force-pushed the whz_dynamic_partition branch from 1233b3b to 8a23015 Compare September 26, 2021 07:59
morningman
morningman approved these changes Sep 28, 2021
View reviewed changes
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Sep 28, 2021
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@morningman morningman merged commit cdf9f9e into apache:master Sep 28, 2021
@morningman morningman mentioned this pull request Oct 10, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morningman morningman morningman approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. area/dynamic-partition kind/feature Categorizes issue or PR as related to a new feature. reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DynamicPartition] support retention for specified period of history time.
3 participants
@Henry2SS @morningman @caiconghui