[HUDI-1929] Support configure KeyGenerator by type #2993

wangxianghu · 2021-05-25T12:50:08Z

…y type

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

Support configure KeyGenerator by type

Verify this pull request

This pull request is already covered by existing tests, such as
org.apache.hudi.keygen.factory.TestHoodieSparkKeyGeneratorFactory
org.apache.hudi.keygen.TestCustomKeyGenerator
org.apache.hudi.utilities.functional.TestHoodieDeltaStreamer

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.

codecov-commenter · 2021-05-26T01:14:21Z

Codecov Report

Merging #2993 (bd59a24) into master (870e97b) will decrease coverage by 15.42%.
The diff coverage is 80.00%.

@@              Coverage Diff              @@
##             master    #2993       +/-   ##
=============================================
- Coverage     70.83%   55.41%   -15.43%     
- Complexity      385     3880     +3495     
=============================================
  Files            54      488      +434     
  Lines          2016    23632    +21616     
  Branches        241     2530     +2289     
=============================================
+ Hits           1428    13095    +11667     
- Misses          454     9369     +8915     
- Partials        134     1168     +1034

Flag	Coverage Δ
hudicli	`39.95% <ø> (?)`
hudiclient	`∅ <ø> (∅)`
hudicommon	`50.30% <ø> (?)`
hudiflink	`63.39% <72.72%> (?)`
hudihadoopmr	`51.43% <ø> (?)`
hudisparkdatasource	`74.30% <100.00%> (?)`
hudisync	`51.32% <ø> (?)`
huditimelineservice	`64.36% <ø> (?)`
hudiutilities	`70.83% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../org/apache/hudi/streamer/FlinkStreamerConfig.java	`0.00% <0.00%> (ø)`
...c/main/java/org/apache/hudi/util/StreamerUtil.java	`57.39% <ø> (ø)`
...va/org/apache/hudi/configuration/FlinkOptions.java	`91.53% <85.71%> (ø)`
...e/hudi/sink/transform/RowDataToHoodieFunction.java	`100.00% <100.00%> (ø)`
...i/bootstrap/SparkParquetBootstrapDataProvider.java	`80.00% <100.00%> (ø)`
...n/scala/org/apache/hudi/HoodieSparkSqlWriter.scala	`70.14% <100.00%> (ø)`
...main/scala/org/apache/hudi/HoodieWriterUtils.scala	`83.33% <100.00%> (ø)`
...apache/hudi/utilities/deltastreamer/DeltaSync.java	`70.84% <100.00%> (ø)`
...a/org/apache/hudi/cli/commands/CommitsCommand.java	`54.83% <0.00%> (ø)`
...ain/java/org/apache/hudi/avro/HoodieAvroUtils.java	`55.66% <0.00%> (ø)`
... and 432 more

wangxianghu · 2021-05-26T05:56:13Z

@yanghua please take a look when free

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java

nsivabalan

left some comments.

nsivabalan

Left a comment on two key gen configs.

nsivabalan

guess we need to fix quite a few more places.

hudi/hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DataSourceOptions.scala

Line 213 in e6a71ea

    
           val keyGeneratorClass = optParams.getOrElse(DataSourceWriteOptions.KEYGENERATOR_CLASS_OPT_KEY,

hudi/hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/DataSourceUtils.java

Line 90 in e6a71ea

    
           public static KeyGenerator createKeyGenerator(TypedProperties props) throws IOException {

Also, I see you have added a keygen factory for spark. what's the plan for Flink ?

hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestCustomKeyGenerator.java

wangxianghu · 2021-05-31T13:54:58Z

guess we need to fix quite a few more places.

hudi/hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DataSourceOptions.scala

Line 213 in e6a71ea

val keyGeneratorClass = optParams.getOrElse(DataSourceWriteOptions.KEYGENERATOR_CLASS_OPT_KEY,

hudi/hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/DataSourceUtils.java

Line 90 in e6a71ea

public static KeyGenerator createKeyGenerator(TypedProperties props) throws IOException {

Also, I see you have added a keygen factory for spark. what's the plan for Flink ?
The keyGenerator of flink is in hudi-client-common, I plan to add a factory for flink and java engine. they can share it

wangxianghu · 2021-05-31T13:58:12Z

few

Yes, several places. I will change them one by one

wangxianghu · 2021-05-31T14:11:21Z

guess we need to fix quite a few more places.

hudi/hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DataSourceOptions.scala

Line 213 in e6a71ea

val keyGeneratorClass = optParams.getOrElse(DataSourceWriteOptions.KEYGENERATOR_CLASS_OPT_KEY,

hudi/hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/DataSourceUtils.java

Line 90 in e6a71ea

public static KeyGenerator createKeyGenerator(TypedProperties props) throws IOException {

Also, I see you have added a keygen factory for spark. what's the plan for Flink ?

Currently, flink and java engine shares key generator, so I plan to add a factory in hudi-client-common to support both of them.

nsivabalan

Were you able to fix all places where keygen is instantiated.

...lient/hudi-client-common/src/main/java/org/apache/hudi/keygen/constant/KeyGeneratorType.java

hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestCustomKeyGenerator.java

nsivabalan · 2021-06-02T11:28:11Z

May I know what do you mean by "Yes, several places. I will change them one by one" ? Do you mean to say, you plan to address in a follow up patch ? IMO, it makes sense to do it in this patch itself.

wangxianghu · 2021-06-02T13:40:47Z

May I know what do you mean by "Yes, several places. I will change them one by one" ? Do you mean to say, you plan to address in a follow up patch ? IMO, it makes sense to do it in this patch itself.

Ok, I'll update this pr to change all of them at once

wangxianghu · 2021-06-03T12:47:23Z

hi @nsivabalan, I have addressed all your concerns, please take a look when free

wangxianghu · 2021-06-05T03:14:11Z

@nsivabalan this pr is ready now, please take a look when free

...lient-common/src/main/java/org/apache/hudi/keygen/factory/HoodieAvroKeyGeneratorFactory.java

...park-client/src/main/java/org/apache/hudi/keygen/factory/HoodieSparkKeyGeneratorFactory.java

nsivabalan

last few minor comments. We can merge this in once addressed.

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java

...lient-common/src/main/java/org/apache/hudi/keygen/factory/HoodieAvroKeyGeneratorFactory.java

...di-client-common/src/test/java/org/apache/hudi/keygen/TestHoodieAvroKeyGeneratorFactory.java

...-client/src/test/java/org/apache/hudi/keygen/factory/TestHoodieSparkKeyGeneratorFactory.java

nsivabalan · 2021-06-06T12:12:32Z

btw, do not forget to update our config page with the new config https://issues.apache.org/jira/browse/HUDI-1983. Please assign the ticket to yourself and take it up once the PR is landed. thanks in advance.

wangxianghu · 2021-06-07T02:31:46Z

@nsivabalan thanks for the detailed review, I have addressed all your concerns.
PTAL

nsivabalan

few minor comments. we are almost there :)

...lient-common/src/main/java/org/apache/hudi/keygen/factory/HoodieAvroKeyGeneratorFactory.java

...rc/test/java/org/apache/hudi/keygen/factory/TestCreateAvroKeyGeneratorByTypeWithFactory.java

nsivabalan · 2021-06-07T11:14:37Z

...nt/src/test/java/org/apache/hudi/keygen/factory/TestCreateKeyGeneratorByTypeWithFactory.java

+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {"SIMPLE", "TIMESTAMP","COMPLEX",  "CUSTOM", "NON_PARTITION", "GLOBAL_DELETE"})


similar comment as above

nsivabalan

LGTM. Will merge once CI succeeds. thanks for patiently addressing all feedback :)

nsivabalan · 2021-06-07T16:18:50Z

Can you check the CI failure.

wangxianghu · 2021-06-08T01:24:05Z

reopen to trigger the ci

wangxianghu · 2021-06-08T11:18:27Z

reopen to trigger the ci

nsivabalan · 2021-06-08T13:26:01Z

Thanks for the contribution :) Will land this in.

wangxianghu force-pushed the HUDI-1929 branch 2 times, most recently from 3a43669 to 89e4fa7 Compare May 26, 2021 01:09

wangxianghu force-pushed the HUDI-1929 branch 3 times, most recently from a065b46 to d0156e3 Compare May 26, 2021 04:47

yanghua self-assigned this May 26, 2021

yanghua reviewed May 28, 2021

View reviewed changes

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java Outdated Show resolved Hide resolved

nsivabalan reviewed May 29, 2021

View reviewed changes

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java Outdated Show resolved Hide resolved

nsivabalan reviewed May 29, 2021

View reviewed changes

nsivabalan reviewed May 31, 2021

View reviewed changes

wangxianghu force-pushed the HUDI-1929 branch from d7a8619 to cc6d699 Compare May 31, 2021 03:38

nsivabalan requested changes May 31, 2021

View reviewed changes

hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestCustomKeyGenerator.java Outdated Show resolved Hide resolved

yanghua assigned nsivabalan and unassigned yanghua Jun 1, 2021

nsivabalan requested changes Jun 2, 2021

View reviewed changes

wangxianghu marked this pull request as draft June 3, 2021 03:31

wangxianghu force-pushed the HUDI-1929 branch from c942ba2 to a2cc546 Compare June 3, 2021 11:36

wangxianghu changed the title ~~[HUDI-1929] Make HoodieDeltaStreamer support configure KeyGenerator by type~~ [HUDI-1929] Support configure KeyGenerator by type Jun 3, 2021

wangxianghu force-pushed the HUDI-1929 branch from 8744e16 to b2941c9 Compare June 3, 2021 12:40

wangxianghu marked this pull request as ready for review June 3, 2021 12:40

wangxianghu force-pushed the HUDI-1929 branch 2 times, most recently from 2c083a4 to 249c67a Compare June 3, 2021 12:46

wangxianghu force-pushed the HUDI-1929 branch 2 times, most recently from bd19c7a to 82ae8d3 Compare June 5, 2021 00:57

wangxianghu marked this pull request as ready for review June 5, 2021 00:57

nsivabalan requested changes Jun 5, 2021

View reviewed changes

PR Tracker Board automation moved this from Under Discussion PRs to Nearing Landing Jun 5, 2021

nsivabalan reviewed Jun 6, 2021

View reviewed changes

nsivabalan reviewed Jun 7, 2021

View reviewed changes

nsivabalan approved these changes Jun 7, 2021

View reviewed changes

wangxianghu closed this Jun 8, 2021

PR Tracker Board automation moved this from Nearing Landing to Done Jun 8, 2021

wangxianghu reopened this Jun 8, 2021

PR Tracker Board automation moved this from Done to Under Discussion PRs Jun 8, 2021

wangxianghu force-pushed the HUDI-1929 branch 2 times, most recently from bd59a24 to c439bb0 Compare June 8, 2021 07:33

[HUDI-1929] Support configure KeyGenerator by type

f21a845

wangxianghu force-pushed the HUDI-1929 branch from c439bb0 to f21a845 Compare June 8, 2021 07:35

wangxianghu closed this Jun 8, 2021

PR Tracker Board automation moved this from Under Discussion PRs to Done Jun 8, 2021

wangxianghu reopened this Jun 8, 2021

PR Tracker Board automation moved this from Done to Under Discussion PRs Jun 8, 2021

nsivabalan merged commit 7261f08 into apache:master Jun 8, 2021

PR Tracker Board automation moved this from Under Discussion PRs to Done Jun 8, 2021

umehrot2 mentioned this pull request Aug 19, 2021

[HUDI-2315] Restore 0.8.0 config keys with deprecated annotation #3491

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HUDI-1929] Support configure KeyGenerator by type #2993

[HUDI-1929] Support configure KeyGenerator by type #2993

wangxianghu commented May 25, 2021 •

edited

codecov-commenter commented May 26, 2021 •

edited

wangxianghu commented May 26, 2021

nsivabalan left a comment

nsivabalan left a comment

nsivabalan left a comment

wangxianghu commented May 31, 2021

wangxianghu commented May 31, 2021

wangxianghu commented May 31, 2021 •

edited

nsivabalan left a comment

nsivabalan commented Jun 2, 2021

wangxianghu commented Jun 2, 2021

wangxianghu commented Jun 3, 2021

wangxianghu commented Jun 5, 2021

nsivabalan left a comment

nsivabalan commented Jun 6, 2021

wangxianghu commented Jun 7, 2021

nsivabalan left a comment

nsivabalan Jun 7, 2021

wangxianghu Jun 7, 2021

nsivabalan left a comment

nsivabalan commented Jun 7, 2021

wangxianghu commented Jun 8, 2021

wangxianghu commented Jun 8, 2021

nsivabalan commented Jun 8, 2021

[HUDI-1929] Support configure KeyGenerator by type #2993

[HUDI-1929] Support configure KeyGenerator by type #2993

Conversation

wangxianghu commented May 25, 2021 • edited

Tips

What is the purpose of the pull request

Verify this pull request

Committer checklist

codecov-commenter commented May 26, 2021 • edited

Codecov Report

wangxianghu commented May 26, 2021

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan left a comment

Choose a reason for hiding this comment

wangxianghu commented May 31, 2021

wangxianghu commented May 31, 2021

wangxianghu commented May 31, 2021 • edited

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan commented Jun 2, 2021

wangxianghu commented Jun 2, 2021

wangxianghu commented Jun 3, 2021

wangxianghu commented Jun 5, 2021

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan commented Jun 6, 2021

wangxianghu commented Jun 7, 2021

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan Jun 7, 2021

Choose a reason for hiding this comment

wangxianghu Jun 7, 2021

Choose a reason for hiding this comment

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan commented Jun 7, 2021

wangxianghu commented Jun 8, 2021

wangxianghu commented Jun 8, 2021

nsivabalan commented Jun 8, 2021

wangxianghu commented May 25, 2021 •

edited

codecov-commenter commented May 26, 2021 •

edited

wangxianghu commented May 31, 2021 •

edited