[HUDI-2397] Add `--enable-sync` parameter #3608

kination · 2021-09-06T11:40:25Z

Tips

Thank you very much for contributing to Apache Hudi.
Please review https://hudi.apache.org/contribute/how-to-contribute before opening a pull request.

What is the purpose of the pull request

HoodieMultiTableDeltaStreamer is just a wrapper on top of HoodieDeltaStreamer and --enable-sync parameter needs to be introduced there so that both the streamer classes are in sync.

Verify this pull request

Add enableMetaSync config for HoodieMultiTableDeltaStreamer, to sync with HoodieDeltaStreamer

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.

hudi-bot · 2021-09-06T11:42:22Z

CI report:

9d7898a Azure: FAILURE

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run travis re-run the last Travis build
@hudi-bot run azure re-run the last Azure build

kination · 2021-09-06T12:00:57Z

@pratyakshsharma is enableMetaSync just a replacement of enableHiveSync, or is there some other changes?

if (cfg.enableHiveSync && StringUtils.isNullOrEmpty(tableProperties.getString(DataSourceWriteOptions.HIVE_TABLE().key(), ""))) {
        throw new HoodieException("Hive sync table field not provided!");
      }

=> Is it okay to replace this to enableMetaSync?

vinothchandar · 2021-09-07T21:20:51Z

@pratyakshsharma there are quite a few requests around the multi delta streamer. Are you still interested in helping improve it and be point person for all feature contribution? please let me know

pratyakshsharma · 2021-09-08T05:31:55Z

@pratyakshsharma there are quite a few requests around the multi delta streamer. Are you still interested in helping improve it and be point person for all feature contribution? please let me know

Big yes to that :) . Please let me know if I can help for anything.

pratyakshsharma · 2021-09-08T05:32:54Z

@djKooks Thank you for raising this. Will check this today.

pratyakshsharma

Is it okay to replace this to enableMetaSync?

Yes we can replace it with enableMetaSync here.

Small comments, rest looks good. :)

pratyakshsharma · 2021-09-11T09:17:40Z

...es/src/test/java/org/apache/hudi/utilities/functional/TestHoodieMultiTableDeltaStreamer.java

@@ -138,7 +139,7 @@ public void testMultiTableExecutionWithKafkaSource() throws IOException {
    testUtils.sendMessages(topicName1, Helpers.jsonifyRecords(dataGenerator.generateInsertsAsPerSchema("000", 5, HoodieTestDataGenerator.TRIP_SCHEMA)));
    testUtils.sendMessages(topicName2, Helpers.jsonifyRecords(dataGenerator.generateInsertsAsPerSchema("000", 10, HoodieTestDataGenerator.SHORT_TRIP_SCHEMA)));

-    HoodieMultiTableDeltaStreamer.Config cfg = TestHelpers.getConfig(PROPS_FILENAME_TEST_SOURCE1, dfsBasePath + "/config", JsonKafkaSource.class.getName(), false);
+    HoodieMultiTableDeltaStreamer.Config cfg = TestHelpers.getConfig(PROPS_FILENAME_TEST_SOURCE1, dfsBasePath + "/config", JsonKafkaSource.class.getName(), false, true);


Let us keep enableMetaSync as false where enableHiveSync is also false? Otherwise it might lead to confusion.

pratyakshsharma · 2021-09-11T09:17:54Z

...es/src/test/java/org/apache/hudi/utilities/functional/TestHoodieMultiTableDeltaStreamer.java

@@ -187,7 +188,7 @@ public void testMultiTableExecutionWithParquetSource() throws IOException {
    // add only common props. later we can add per table props
    String parquetPropsFile = populateCommonPropsAndWriteToFile();

-    HoodieMultiTableDeltaStreamer.Config cfg = TestHelpers.getConfig(parquetPropsFile, dfsBasePath + "/config", ParquetDFSSource.class.getName(), false,
+    HoodieMultiTableDeltaStreamer.Config cfg = TestHelpers.getConfig(parquetPropsFile, dfsBasePath + "/config", ParquetDFSSource.class.getName(), false, true,


pratyakshsharma · 2021-09-11T09:18:03Z

...es/src/test/java/org/apache/hudi/utilities/functional/TestHoodieMultiTableDeltaStreamer.java

@@ -218,7 +219,7 @@ public void testMultiTableExecutionWithParquetSource() throws IOException {

  @Test
  public void testTableLevelProperties() throws IOException {
-    HoodieMultiTableDeltaStreamer.Config cfg = TestHelpers.getConfig(PROPS_FILENAME_TEST_SOURCE1, dfsBasePath + "/config", TestDataSource.class.getName(), false);
+    HoodieMultiTableDeltaStreamer.Config cfg = TestHelpers.getConfig(PROPS_FILENAME_TEST_SOURCE1, dfsBasePath + "/config", TestDataSource.class.getName(), false, true);


kination · 2021-09-13T00:17:43Z

@pratyakshsharma thanks for comment. Updated 🙏

pratyakshsharma · 2021-09-13T05:55:57Z

LGTM. Will merge it once CI passes.

Thank you for your contribution @djKooks

kination · 2021-09-13T06:29:29Z

@pratyakshsharma thanks for help~please share if there's other features needs to be improved/fixed 🙏
(will keep look-on the project)

pratyakshsharma · 2021-09-13T06:34:26Z

@pratyakshsharma thanks for help~please share if there's other features needs to be improved/fixed pray
(will keep look-on the project)

Definitely! You can search for jiras with newbie label and pick them up one by one :)

vinothchandar · 2021-09-13T13:38:35Z

@pratyakshsharma This broke master due to a failing test. Please be careful about merging. by ensuring the CI in azure actually passes before you land.

https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build/results?buildId=2180&view=logs&j=b1544eb9-7ff1-5db9-0187-3e05abf459bc&t=e0ae894b-41c9-5f4b-7ed2-bdf5243b02e7

#3654 I am going to land this soon, which I think fixes this.

add meta-sync config

d4abba5

update test

8227a3c

vinothchandar assigned pratyakshsharma Sep 7, 2021

pratyakshsharma requested changes Sep 11, 2021

View reviewed changes

kination marked this pull request as ready for review September 12, 2021 06:02

keep enableMetaSync same with enableHiveSync

3e0f34c

Switch check logic to use enableMetaSync

9d7898a

pratyakshsharma approved these changes Sep 13, 2021

View reviewed changes

pratyakshsharma merged commit c79017c into apache:master Sep 13, 2021

kination deleted the jung/multi-table-streamer-enablesync branch September 13, 2021 06:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HUDI-2397] Add `--enable-sync` parameter #3608

[HUDI-2397] Add `--enable-sync` parameter #3608

kination commented Sep 6, 2021 •

edited

Loading

hudi-bot commented Sep 6, 2021 •

edited

Loading

kination commented Sep 6, 2021

vinothchandar commented Sep 7, 2021

pratyakshsharma commented Sep 8, 2021

pratyakshsharma commented Sep 8, 2021

pratyakshsharma left a comment

pratyakshsharma Sep 11, 2021

pratyakshsharma Sep 11, 2021

pratyakshsharma Sep 11, 2021

kination commented Sep 13, 2021

pratyakshsharma commented Sep 13, 2021

kination commented Sep 13, 2021 •

edited

Loading

pratyakshsharma commented Sep 13, 2021 •

edited

Loading

vinothchandar commented Sep 13, 2021

[HUDI-2397] Add --enable-sync parameter #3608

[HUDI-2397] Add --enable-sync parameter #3608

Conversation

kination commented Sep 6, 2021 • edited Loading

Tips

What is the purpose of the pull request

Verify this pull request

Committer checklist

hudi-bot commented Sep 6, 2021 • edited Loading

CI report:

kination commented Sep 6, 2021

vinothchandar commented Sep 7, 2021

pratyakshsharma commented Sep 8, 2021

pratyakshsharma commented Sep 8, 2021

pratyakshsharma left a comment

Choose a reason for hiding this comment

pratyakshsharma Sep 11, 2021

Choose a reason for hiding this comment

pratyakshsharma Sep 11, 2021

Choose a reason for hiding this comment

pratyakshsharma Sep 11, 2021

Choose a reason for hiding this comment

kination commented Sep 13, 2021

pratyakshsharma commented Sep 13, 2021

kination commented Sep 13, 2021 • edited Loading

pratyakshsharma commented Sep 13, 2021 • edited Loading

vinothchandar commented Sep 13, 2021

[HUDI-2397] Add `--enable-sync` parameter #3608

[HUDI-2397] Add `--enable-sync` parameter #3608

kination commented Sep 6, 2021 •

edited

Loading

hudi-bot commented Sep 6, 2021 •

edited

Loading

kination commented Sep 13, 2021 •

edited

Loading

pratyakshsharma commented Sep 13, 2021 •

edited

Loading