[DynamicPartition] Optimize the rule of creating dynamic partition #3679

morningman · 2020-05-25T06:51:14Z

Problem is described in ISSUE #3678
This CL mainly changed to rule of creating dynamic partition.

If time unit is DAY, the logic remains unchanged.
If time unit is WEEK, the logical changes are as follows:
1. Allow to set the start day of every week, the default is Monday. Optional Monday to Sunday
2. Assuming that the starting day is a Tuesday, the range of the partition is Tuesday of the week to Monday of the next week.
If time unit is MONTH, the logical changes are as follows:
1. Allow to set the start date of each month. The default is 1st, and can be selected from 1st to 28th.
2. Assuming that the starting date is the 2nd, the range of the partition is from the 2nd of this month to the 1st of the next month.
The SHOW DYNAMIC PARTITION TABLES statement adds a StartOf column to show the start day of week or month.

It is recommended to refer to the example in dynamic-partition.md to understand.

TODO:
Better to support HOUR and YEAR time unit. Maybe in next PR.

WingsGo · 2020-05-25T08:22:18Z

docs/zh-CN/administrator-guide/dynamic-partition.md

 * dynamic\_partition\_enable

-    是否开启 Doris 的动态分区功能。默认为 false，即关闭。该参数只影响动态分区表的分区操作，不影响普通表。
+    是否开启 Doris 的动态分区功能。默认为 false，即关闭。该参数只影响动态分区表的分区操作，不影响普通表。可以通过修改 fe.conf 中的参数并重启 FE 生效。也可以在运行时执行以下命令生效：


这里我们是否可以放开默认为true?目前已经存在了比较长的时间了，有些用户可能不仔细看文档，创建了分区表但是没有修改配置，造成使用上的一些额外成本？

我打算等0.12版本稳定了再打开这个限制，目前来看，还是有各种各样的小问题。。

wutiangan · 2020-05-25T08:27:52Z

docs/zh-CN/administrator-guide/dynamic-partition.md

+p20200521: ["2020-05-21", "2020-05-22")
 ```

+如果此时将分区粒度改为 MONTH，则系统会尝试创建范围为 `["2020-05-01", "2020-06-01")` 的分区，而该分区的分区范围和已有分区冲突，所以无法创建。而范围为 `["2020-06-01", "2020-07-01")` 的分区可以正常创建。因此，2020-05-22 到 2020-05-30 时间段的分区，需要自行填补。


如果命令冲突时，该命令返回成功还是失败？如果返回成功，那些需要补齐的分区能否自动帮助用户补齐？

目前是直接成功，不会报错，show dynamic partition 可以看到错误信息。
这个我建议由用户自行处理。系统来处理这个问题，太复杂了，容易出错，并且隐含的操作太多。

wutiangan · 2020-05-25T08:31:01Z

docs/zh-CN/administrator-guide/dynamic-partition.md

-        
-        返回：200
-        ```
+    `curl --location-trusted -u username:password -XGET http://fe_host:fe_http_port/api/_set_config?dynamic_partition_check_interval_seconds=432000`


设置http协议为啥用GET方法而不是SET方法？

这个就是沿用之前的框架，没有做更多的修改

WingsGo · 2020-05-25T08:33:21Z

fe/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java

+            throw new DdlException("Invalid properties: " + DynamicPartitionProperty.START_DAY_OF_WEEK);
+        }
+        try {
+            int dayOfMonth = Integer.parseInt(val);


wutiangan · 2020-05-25T08:40:54Z

fe/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java

+        // 2. get the date of `startOf` week
+        int day = current.get(Calendar.DAY_OF_WEEK);
+        // SUNDAY will return 1, we will set it to 7, and make MONDAY to 1, and so on
+        day = day == 1 ? 7 : day - 1;


you‘d better add Parentheses

wutiangan · 2020-05-25T08:42:39Z

fe/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java

+     *  Today is 2020-05-24, offset = 1, startOf.month = 3
+     *  It will return 2020-06-03 
+     */
+    private static String getPartitionRangeOfMonth(Calendar current, int offset, StartOfDate startOf, TimeZone tz,


why tz is no use?

What about use LocalDateTime/ZonedDateTime to replace all Calendar? They are thread-safe, immutable and support timezone

Maybe in next PR? This PR is already too big

OK, we can change it in next PR~

WingsGo · 2020-05-25T10:07:34Z

fe/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java

+        current.add(Calendar.WEEK_OF_YEAR, offset);
+        // 2. get the date of `startOf` week
+        int day = current.get(Calendar.DAY_OF_WEEK);
+        // SUNDAY will return 1, we will set it to 7, and make MONDAY to 1, and so on


Use LocalDateTime can avoid this trick

WingsGo · 2020-05-25T10:12:02Z

fe/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java

+        }
+    }
+
+    /*


java doc comment is start with /** xx */ , maybe we can unify it?

Ok, I will change my IDE's setting. But there are lots of comment write like this. I can just make sure that newly added code can obey to rule.

It looks good.

WingsGo · 2020-05-25T10:30:24Z

fe/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java

            // keys.size is always equal to column.size, cannot reach this exception
-            LOG.warn("Keys size is not equal to column size. Error={}", e.getMessage());
+            LOG.warn("Keys size is not equal to column size. Error={}, db: {}, table: {}", e.getMessage(),
+                    db.getFullName(), olapTable.getName());


maybe we should remove the warning, because dynamic_partition.start is default to Integer.MIN_VALUE, so we may got a AnalysisException like this Error=errCode = 2, detailMessage = date literal [5877471-07-26 00:00:00] is invalid

OK, I will check this offset to make it reasonable.

I just make a check that if start offset == Integer.MIN_VALUE, it will not drop any partition.
And for adding partition, I just leave it to user to make sure the value is reasonable.

WingsGo · 2020-05-25T11:59:02Z

fe/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java

                // keys.size is always equal to column.size, cannot reach this exception
-                LOG.warn("Keys size is not equal to column size. Error={}", e.getMessage());
+                LOG.warn("Keys size is not equal to column size. Error={}, db: {}, table: {}", e.getMessage(),
+                        db.getFullName(), olapTable.getName());


same as following comment

WingsGo · 2020-05-25T12:00:26Z

fe/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java

+                    iterator.remove();
+                    continue;
+                }
+


The single column check work is done when create table, no need to check here

It is just a self-defence~

WingsGo · 2020-05-25T12:01:52Z

fe/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java

+        }
+        try {
+            int dayOfMonth = Integer.parseInt(val);
+            if (dayOfMonth < 1 || dayOfMonth > 28) {


add a comment why less than 28, the reason in doc is for users?

I have already stated the reason in the document, I will add some comments here.

WingsGo · 2020-05-26T02:11:30Z

fe/src/main/java/org/apache/doris/common/util/TimeUtils.java

+    // return the number of days of give year.month
+    public static int getNumberDaysOfMonth(int year, int month) {
+        Calendar calendar = Calendar.getInstance();
+        calendar.set(year, month + 1, 0);


why plus one? getNumberDaysOfMonth(2020, 1) will return 29, and the function name is confused

This function is useless, i will remove it

WingsGo · 2020-05-26T11:40:18Z

LGTM

…pache#3679) Problem is described in ISSUE apache#3678 This CL mainly changed to rule of creating dynamic partition. 1. If time unit is DAY, the logic remains unchanged. 2. If time unit is WEEK, the logical changes are as follows: 1. Allow to set the start day of every week, the default is Monday. Optional Monday to Sunday 2. Assuming that the starting day is a Tuesday, the range of the partition is Tuesday of the week to Monday of the next week. 3. If time unit is MONTH, the logical changes are as follows: 1. Allow to set the start date of each month. The default is 1st, and can be selected from 1st to 28th. 2. Assuming that the starting date is the 2nd, the range of the partition is from the 2nd of this month to the 1st of the next month. 4. The `SHOW DYNAMIC PARTITION TABLES` statement adds a `StartOf` column to show the start day of week or month. It is recommended to refer to the example in `dynamic-partition.md` to understand. TODO: Better to support HOUR and YEAR time unit. Maybe in next PR. FIX: apache#3678

morningman added 5 commits May 25, 2020 14:31

first

2c862e8

add doc

c78b750

add ut

7545b5c

add show info

5dd0738

modify doc

90faac2

morningman added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API area/catalog Issues or PRs related to catalog management labels May 25, 2020

morningman self-assigned this May 25, 2020

WingsGo reviewed May 25, 2020

View reviewed changes

wutiangan reviewed May 25, 2020

View reviewed changes

WingsGo reviewed May 25, 2020

View reviewed changes

wutiangan reviewed May 25, 2020

View reviewed changes

WingsGo reviewed May 25, 2020

View reviewed changes

fix bug

0d23693

WingsGo reviewed May 25, 2020

View reviewed changes

fix by review

82a3cee

WingsGo reviewed May 26, 2020

View reviewed changes

fix by review

888d123

add transaction not found

2dc45ea

EmmyMiao87 approved these changes May 27, 2020

View reviewed changes

morningman merged commit bc35f3a into apache:master May 27, 2020

WingsGo mentioned this pull request Jun 17, 2020

[Dynamic Partition] DynamicPartitionScheduler.getDropPartitionClause WARN because dynamic_partition.start not set ? #3892

Closed

EmmyMiao87 mentioned this pull request Aug 17, 2020

Release Notes 0.13.0 #4370

Closed

+                      }
+                  }
+                  /*

[DynamicPartition] Optimize the rule of creating dynamic partition #3679

[DynamicPartition] Optimize the rule of creating dynamic partition #3679

Uh oh!

Conversation

morningman commented May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wutiangan May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WingsGo May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WingsGo May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WingsGo May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WingsGo commented May 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

morningman commented May 25, 2020 •

edited

Loading

wutiangan May 25, 2020 •

edited

Loading

WingsGo May 25, 2020 •

edited

Loading

WingsGo May 25, 2020 •

edited

Loading

WingsGo May 25, 2020 •

edited

Loading