You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/09/27 08:15:58 UTC
[GitHub] [hudi] liujinhui1994 opened a new pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
liujinhui1994 opened a new pull request #2122:
URL: https://github.com/apache/hudi/pull/2122
## *Tips*
- *Thank you very much for contributing to Apache Hudi.*
- *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
## What is the purpose of the pull request
Make hive synchronization supports hourly partition
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] liujinhui1994 commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
liujinhui1994 commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r512697102
##########
File path: hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/PartitionValueExtractorTest.java
##########
@@ -0,0 +1,17 @@
+package org.apache.hudi.hive;
+
+import org.junit.jupiter.api.Test;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class PartitionValueExtractorTest {
+ @Test
+ public void testHourPartition() {
Review comment:
ok
##########
File path: hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/SlashEncodedHourPartitionValueExtractor.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.hive;
+
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * HDFS Path contain hive partition values for the keys it is partitioned on. This mapping is not straight forward and
+ * requires a pluggable implementation to extract the partition value from HDFS path.
+ * <p>
+ * This implementation extracts datestr=yyyy-mm-dd-HH from path of type /yyyy/mm/dd/HH
+ */
+public class SlashEncodedHourPartitionValueExtractor implements PartitionValueExtractor {
+
+ private static final long serialVersionUID = 1L;
+ private transient DateTimeFormatter dtfOut;
+
+ public SlashEncodedHourPartitionValueExtractor() {
+ this.dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd-HH");
+ }
+
+ private DateTimeFormatter getDtfOut() {
+ if (dtfOut == null) {
+ dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd-HH");
+ }
+ return dtfOut;
+ }
+
+ @Override
+ public List<String> extractPartitionValuesInPath(String partitionPath) {
+ // partition path is expected to be in this format yyyy/mm/dd/HH
+ String[] splits = partitionPath.split("/");
+ if (splits.length != 4) {
+ throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd/HH");
+ }
+ // Get the partition part and remove the / as well at the end
Review comment:
ok
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codecov-io commented on pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-706100867
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=h1) Report
> Merging [#2122](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/3a578d740234db9c44e6b9f8d27d21713eb5f386?el=desc) will **decrease** coverage by `7.85%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2122 +/- ##
============================================
- Coverage 61.59% 53.74% -7.86%
+ Complexity 4016 568 -3448
============================================
Files 508 94 -414
Lines 20757 4196 -16561
Branches 2032 471 -1561
============================================
- Hits 12786 2255 -10531
+ Misses 7057 1736 -5321
+ Partials 914 205 -709
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (-26.19%)` | `193.00 <ø> (-1498.00)` | |
| #hudiclient | `?` | `?` | |
| #hudicommon | `?` | `?` | |
| #hudispark | `?` | `?` | |
| #huditimelineservice | `62.29% <ø> (-1.19%)` | `50.00 <ø> (+3.00)` | :arrow_down: |
| #hudiutilities | `69.98% <ø> (+0.84%)` | `325.00 <ø> (+30.00)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...udi/utilities/schema/DelegatingSchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9EZWxlZ2F0aW5nU2NoZW1hUHJvdmlkZXIuamF2YQ==) | `75.00% <0.00%> (-25.00%)` | `3.00% <0.00%> (ø%)` | |
| [.../apache/hudi/timeline/service/TimelineService.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvVGltZWxpbmVTZXJ2aWNlLmphdmE=) | `25.00% <0.00%> (-2.54%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...udi/utilities/sources/helpers/DFSPathSelector.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9ERlNQYXRoU2VsZWN0b3IuamF2YQ==) | `82.05% <0.00%> (-2.16%)` | `12.00% <0.00%> (ø%)` | |
| [...a/org/apache/hudi/cli/commands/RepairsCommand.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1JlcGFpcnNDb21tYW5kLmphdmE=) | `69.33% <0.00%> (-1.90%)` | `10.00% <0.00%> (ø%)` | |
| [...i/utilities/deltastreamer/SourceFormatAdapter.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvU291cmNlRm9ybWF0QWRhcHRlci5qYXZh) | `86.48% <0.00%> (-0.70%)` | `11.00% <0.00%> (ø%)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `88.77% <0.00%> (-0.64%)` | `16.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...ties/deltastreamer/HoodieDeltaStreamerMetrics.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lck1ldHJpY3MuamF2YQ==) | `36.11% <0.00%> (-0.26%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...ain/scala/org/apache/hudi/cli/DedupeSparkJob.scala](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9odWRpL2NsaS9EZWR1cGVTcGFya0pvYi5zY2FsYQ==) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...rg/apache/hudi/cli/commands/SavepointsCommand.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1NhdmVwb2ludHNDb21tYW5kLmphdmE=) | `14.28% <0.00%> (ø)` | `3.00% <0.00%> (ø%)` | |
| [...a/org/apache/hudi/utilities/sources/RowSource.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvUm93U291cmNlLmphdmE=) | `100.00% <0.00%> (ø)` | `4.00% <0.00%> (ø%)` | |
| ... and [435 more](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] liujinhui1994 commented on pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
liujinhui1994 commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-716262613
Sorry, I am late in reply, I will add unit tests soon
@vinothchandar
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r496439235
##########
File path: hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/SlashEncodedDayPartitionValueExtractor.java
##########
@@ -36,30 +36,35 @@
private static final long serialVersionUID = 1L;
private transient DateTimeFormatter dtfOut;
- public SlashEncodedDayPartitionValueExtractor() {
Review comment:
If you try to support hourly partition, then this name is not suitable. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r495659569
##########
File path: hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/SlashEncodedDayPartitionValueExtractor.java
##########
@@ -36,30 +36,40 @@
private static final long serialVersionUID = 1L;
private transient DateTimeFormatter dtfOut;
- public SlashEncodedDayPartitionValueExtractor() {
- this.dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd");
- }
-
- private DateTimeFormatter getDtfOut() {
- if (dtfOut == null) {
- dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd");
- }
- return dtfOut;
- }
-
@Override
public List<String> extractPartitionValuesInPath(String partitionPath) {
+
+ int year = 0;
+ int mm = 0;
+ int dd = 0;
+ int hh = 0;
// partition path is expected to be in this format yyyy/mm/dd
String[] splits = partitionPath.split("/");
- if (splits.length != 3) {
- throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd ");
+ if (splits.length != 3 && splits.length != 4) {
+ throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd or yyyy/mm/dd/HH");
}
// Get the partition part and remove the / as well at the end
- int year = Integer.parseInt(splits[0].contains("=") ? splits[0].split("=")[1] : splits[0]);
- int mm = Integer.parseInt(splits[1].contains("=") ? splits[1].split("=")[1] : splits[1]);
- int dd = Integer.parseInt(splits[2].contains("=") ? splits[2].split("=")[1] : splits[2]);
- DateTime dateTime = new DateTime(year, mm, dd, 0, 0);
+ if (splits.length == 3 || splits.length == 4) {
Review comment:
This check can be removed, it is always true.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r495775215
##########
File path: hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/SlashEncodedDayPartitionValueExtractor.java
##########
@@ -36,30 +36,35 @@
private static final long serialVersionUID = 1L;
private transient DateTimeFormatter dtfOut;
- public SlashEncodedDayPartitionValueExtractor() {
- this.dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd");
- }
-
- private DateTimeFormatter getDtfOut() {
- if (dtfOut == null) {
- dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd");
- }
- return dtfOut;
- }
-
@Override
public List<String> extractPartitionValuesInPath(String partitionPath) {
+
// partition path is expected to be in this format yyyy/mm/dd
String[] splits = partitionPath.split("/");
- if (splits.length != 3) {
- throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd ");
+ if (splits.length != 3 && splits.length != 4) {
+ throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd or yyyy/mm/dd/HH");
}
// Get the partition part and remove the / as well at the end
int year = Integer.parseInt(splits[0].contains("=") ? splits[0].split("=")[1] : splits[0]);
int mm = Integer.parseInt(splits[1].contains("=") ? splits[1].split("=")[1] : splits[1]);
int dd = Integer.parseInt(splits[2].contains("=") ? splits[2].split("=")[1] : splits[2]);
- DateTime dateTime = new DateTime(year, mm, dd, 0, 0);
+ int hh = 0;
Review comment:
@liujinhui1994 Thanks for addressing my concern, LGTM now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua merged pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #2122:
URL: https://github.com/apache/hudi/pull/2122
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-716069944
I will let @yanghua drive this!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r495663737
##########
File path: hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/SlashEncodedDayPartitionValueExtractor.java
##########
@@ -36,30 +36,40 @@
private static final long serialVersionUID = 1L;
private transient DateTimeFormatter dtfOut;
- public SlashEncodedDayPartitionValueExtractor() {
- this.dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd");
- }
-
- private DateTimeFormatter getDtfOut() {
- if (dtfOut == null) {
- dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd");
- }
- return dtfOut;
- }
-
@Override
public List<String> extractPartitionValuesInPath(String partitionPath) {
+
+ int year = 0;
+ int mm = 0;
+ int dd = 0;
+ int hh = 0;
// partition path is expected to be in this format yyyy/mm/dd
String[] splits = partitionPath.split("/");
- if (splits.length != 3) {
- throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd ");
+ if (splits.length != 3 && splits.length != 4) {
+ throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd or yyyy/mm/dd/HH");
}
// Get the partition part and remove the / as well at the end
- int year = Integer.parseInt(splits[0].contains("=") ? splits[0].split("=")[1] : splits[0]);
- int mm = Integer.parseInt(splits[1].contains("=") ? splits[1].split("=")[1] : splits[1]);
- int dd = Integer.parseInt(splits[2].contains("=") ? splits[2].split("=")[1] : splits[2]);
- DateTime dateTime = new DateTime(year, mm, dd, 0, 0);
+ if (splits.length == 3 || splits.length == 4) {
Review comment:
The rest LGTM cc @yanghua
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] liujinhui1994 commented on pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
liujinhui1994 commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-700452276
@yanghua please help review，thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r513320519
##########
File path: hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestPartitionValueExtractor.java
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.hive;
+
+import org.junit.jupiter.api.Test;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class TestPartitionValueExtractor {
+ @Test
+ public void testHourPartition() {
+ SlashEncodedHourPartitionValueExtractor hourPartition = new SlashEncodedHourPartitionValueExtractor();
+ List<String> list = new ArrayList<>();
+ list.add("2020-12-20-01");
+ assertEquals(hourPartition.extractPartitionValuesInPath("2020/12/20/01"), list);
+ assertThrows(IllegalArgumentException.class, () -> hourPartition.extractPartitionValuesInPath("2020/12/20"));
Review comment:
IMO, it would also be better to check the hive style partitions (good case and bad case). wdyt?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codecov-io commented on pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-706100867
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=h1) Report
> Merging [#2122](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/3a578d740234db9c44e6b9f8d27d21713eb5f386?el=desc) will **decrease** coverage by `7.85%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2122 +/- ##
============================================
- Coverage 61.59% 53.74% -7.86%
+ Complexity 4016 568 -3448
============================================
Files 508 94 -414
Lines 20757 4196 -16561
Branches 2032 471 -1561
============================================
- Hits 12786 2255 -10531
+ Misses 7057 1736 -5321
+ Partials 914 205 -709
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (-26.19%)` | `193.00 <ø> (-1498.00)` | |
| #hudiclient | `?` | `?` | |
| #hudicommon | `?` | `?` | |
| #hudispark | `?` | `?` | |
| #huditimelineservice | `62.29% <ø> (-1.19%)` | `50.00 <ø> (+3.00)` | :arrow_down: |
| #hudiutilities | `69.98% <ø> (+0.84%)` | `325.00 <ø> (+30.00)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2122?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...udi/utilities/schema/DelegatingSchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9EZWxlZ2F0aW5nU2NoZW1hUHJvdmlkZXIuamF2YQ==) | `75.00% <0.00%> (-25.00%)` | `3.00% <0.00%> (ø%)` | |
| [.../apache/hudi/timeline/service/TimelineService.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvVGltZWxpbmVTZXJ2aWNlLmphdmE=) | `25.00% <0.00%> (-2.54%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...udi/utilities/sources/helpers/DFSPathSelector.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9ERlNQYXRoU2VsZWN0b3IuamF2YQ==) | `82.05% <0.00%> (-2.16%)` | `12.00% <0.00%> (ø%)` | |
| [...a/org/apache/hudi/cli/commands/RepairsCommand.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1JlcGFpcnNDb21tYW5kLmphdmE=) | `69.33% <0.00%> (-1.90%)` | `10.00% <0.00%> (ø%)` | |
| [...i/utilities/deltastreamer/SourceFormatAdapter.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvU291cmNlRm9ybWF0QWRhcHRlci5qYXZh) | `86.48% <0.00%> (-0.70%)` | `11.00% <0.00%> (ø%)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `88.77% <0.00%> (-0.64%)` | `16.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...ties/deltastreamer/HoodieDeltaStreamerMetrics.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lck1ldHJpY3MuamF2YQ==) | `36.11% <0.00%> (-0.26%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...ain/scala/org/apache/hudi/cli/DedupeSparkJob.scala](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9odWRpL2NsaS9EZWR1cGVTcGFya0pvYi5zY2FsYQ==) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...rg/apache/hudi/cli/commands/SavepointsCommand.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1NhdmVwb2ludHNDb21tYW5kLmphdmE=) | `14.28% <0.00%> (ø)` | `3.00% <0.00%> (ø%)` | |
| [...a/org/apache/hudi/utilities/sources/RowSource.java](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvUm93U291cmNlLmphdmE=) | `100.00% <0.00%> (ø)` | `4.00% <0.00%> (ø%)` | |
| ... and [435 more](https://codecov.io/gh/apache/hudi/pull/2122/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on pull request #2122: [WIP] [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-699718019
> @wangxianghu Would you please help to review this PR?
sure will do
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2122: [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#discussion_r512375913
##########
File path: hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/PartitionValueExtractorTest.java
##########
@@ -0,0 +1,17 @@
+package org.apache.hudi.hive;
+
+import org.junit.jupiter.api.Test;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class PartitionValueExtractorTest {
+ @Test
+ public void testHourPartition() {
Review comment:
IMO, we need to add more test cases to improve the test coverage. wdyt?
##########
File path: hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/SlashEncodedHourPartitionValueExtractor.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.hive;
+
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * HDFS Path contain hive partition values for the keys it is partitioned on. This mapping is not straight forward and
+ * requires a pluggable implementation to extract the partition value from HDFS path.
+ * <p>
+ * This implementation extracts datestr=yyyy-mm-dd-HH from path of type /yyyy/mm/dd/HH
+ */
+public class SlashEncodedHourPartitionValueExtractor implements PartitionValueExtractor {
+
+ private static final long serialVersionUID = 1L;
+ private transient DateTimeFormatter dtfOut;
+
+ public SlashEncodedHourPartitionValueExtractor() {
+ this.dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd-HH");
+ }
+
+ private DateTimeFormatter getDtfOut() {
+ if (dtfOut == null) {
+ dtfOut = DateTimeFormat.forPattern("yyyy-MM-dd-HH");
+ }
+ return dtfOut;
+ }
+
+ @Override
+ public List<String> extractPartitionValuesInPath(String partitionPath) {
+ // partition path is expected to be in this format yyyy/mm/dd/HH
+ String[] splits = partitionPath.split("/");
+ if (splits.length != 4) {
+ throw new IllegalArgumentException("Partition path " + partitionPath + " is not in the form yyyy/mm/dd/HH");
+ }
+ // Get the partition part and remove the / as well at the end
Review comment:
`remove the / as well at the end` where? And why check contains `=`? It would be better to add more description.
##########
File path: hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/PartitionValueExtractorTest.java
##########
@@ -0,0 +1,17 @@
+package org.apache.hudi.hive;
+
+import org.junit.jupiter.api.Test;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class PartitionValueExtractorTest {
+ @Test
+ public void testHourPartition() {
Review comment:
e.g. some bad cases
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on pull request #2122: [WIP] [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-699712480
@wangxianghu Would you please help to review this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] liujinhui1994 commented on pull request #2122: [WIP] [HUDI-1274] Make hive synchronization supports hourly partition
Posted by GitBox <gi...@apache.org>.
liujinhui1994 commented on pull request #2122:
URL: https://github.com/apache/hudi/pull/2122#issuecomment-699625048
@yanghua please help review
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org