[HUDI-2491] hoodie.datasource.hive_sync.mode=hms mode is supported in… #3799

fuyun2024 · 2021-10-14T08:47:01Z

… spark writer option

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

(For example: This pull request adds quick-start document.)

Brief change log

(for example:)

Modify AnnotationLocation checkstyle rule in checkstyle.xml

Verify this pull request

(Please pick either of the following options)

This pull request is a trivial rework / code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end.
Added HoodieClientWriteTest to verify the change.
Manually verified the change by running a job locally.

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.

… spark writer option

fuyun2024 · 2021-10-14T08:54:21Z

@codepe Are you free to take a look at it for me ? This is my new commit .

codope

@fuyun2024 Thanks for reworking on this. Left a few comments.

codope · 2021-10-14T13:23:42Z

hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/DeltaSync.java

    HiveConf hiveConf = new HiveConf(conf, HiveConf.class);
+    if (!DataSourceWriteOptions.METASTORE_URIS().defaultValue().equals(hiveSyncConfig.metastoreUris)) {


Why do we need to check this? Why not simply set whatever user has passed irrespective of the default value?

codope · 2021-10-14T13:24:14Z

hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/BootstrapExecutor.java

@@ -159,10 +159,14 @@ public void execute() throws IOException {
   * Sync to Hive.
   */
  private void syncHive() {
-    if (cfg.enableHiveSync) {
+    if (cfg.enableHiveSync || cfg.enableHiveSync) {


Redundant condition. Did you mean to add some other condition?

codope · 2021-10-14T13:28:25Z

hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java

@@ -49,6 +49,9 @@
  @Parameter(names = {"--jdbc-url"}, description = "Hive jdbc connect url")
  public String jdbcUrl;

+  @Parameter(names = {"--metastore-uris"}, description = "Hive metastore uris")
+  public String metastoreUris;


This should be used somewhere right. I mean in HoodieHiveClient or HiveSyncTool. I don't see it is being used anywhere in hudi-hive-sync module.

correction parameters

…ies/deltastreamer/DeltaSync.java to hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastream

fuyun2024 · 2021-10-26T12:00:51Z

@codope @vinothchandar anything else should i do, to achive this commit

codope

@fuyun2024 Why is DeltaSync.java renamed?

hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastream

…ream to hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/DeltaSync.java

fuyun2024 · 2021-10-26T12:46:26Z

it was my first time modifying PR online in github, not familiar enough with the operation. I correct it a moment ago, please check it again.

codope · 2021-10-29T15:01:12Z

@hudi-bot run azure

codope

@fuyun2024 Overall, the code is in better shape. One issue is compatibility: users previously passed metastore uri in the same jdbc uri config even in hms mode. Now that the patch introduces a new config for metastore uri, we should ensure that it is backwards compatible with older config as well. Maybe, give precedence to metastore uri and if it's not available then also check jdbc uri. That way existing pipelines won't break if Hudi is upgraded. What do you think?

fuyun2024 · 2021-11-02T01:56:59Z

@codope Thank you for your comments. Yes, I looked at the code and judged that it is compatible in houdiehiveclient()

nsivabalan · 2021-11-02T03:16:51Z

@fuyun2024 : looks like there are some conflicts with master. Can you rebase with latest.

fuyun2024 · 2021-11-02T11:18:59Z

@nsivabalan Yes, with pleasure

codope · 2021-11-03T09:31:06Z

@hudi-bot run azure

codope · 2021-11-03T12:59:39Z

@fuyun2024 Looks like this change is breaking a few tests that make use of HoodieHiveSync in deltastreamer. Can you please check that?

[ERROR] Errors: 
[ERROR]   TestHoodieDeltaStreamer.testBulkInsertsAndUpsertsWithSQLBasedTransformerFor2StepPipeline:1240 » HoodieHiveSync
[ERROR]   TestHoodieDeltaStreamer.testPayloadClassUpdate:1328 » HoodieHiveSync Got runti...
[ERROR]   TestHoodieDeltaStreamer.testPayloadClassUpdateWithCOWTable:1354 » HoodieHiveSync

fuyun2024 · 2021-11-04T15:42:34Z

@codope
hello codope, I'm sorry, I'm busy these days and can't reply in time.
I carefully checked the Java CI log and found no error. Can you tell me where the error is and the error stack information

hudi-bot · 2021-11-05T02:58:38Z

CI report:

aa02b35 UNKNOWN
e98d19e Azure: FAILURE Azure: FAILURE Azure: FAILURE

Bot commands

@hudi-bot supports the following commands:

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

[HUDI-2491] hoodie.datasource.hive_sync.mode=hms mode is supported in…

f44907a

… spark writer option

codope reviewed Oct 14, 2021

View reviewed changes

Update BootstrapExecutor.java

a8b2f1a

correction parameters

vinothchandar self-assigned this Oct 15, 2021

vinothchandar added this to Nearing Landing in PR Tracker Board Oct 15, 2021

fuyun2024 added 3 commits October 17, 2021 21:51

Update and rename hudi-utilities/src/main/java/org/apache/hudi/utilit…

7d49ca0

…ies/deltastreamer/DeltaSync.java to hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastream

Update BootstrapExecutor.java

f33c499

Update HoodieSparkSqlWriter.scala

c04a9ee

codope reviewed Oct 26, 2021

View reviewed changes

hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastream Outdated Show resolved Hide resolved

fuyun2024 added 2 commits October 26, 2021 20:31

Update deltastream

aa02b35

Rename hudi-utilities/src/main/java/org/apache/hudi/utilities/deltast…

e98d19e

…ream to hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/DeltaSync.java

codope reviewed Oct 29, 2021

View reviewed changes

fuyun2024 closed this Nov 5, 2021

fuyun2024 deleted the HUDI-2491 branch November 5, 2021 15:32

PR Tracker Board automation moved this from Nearing Landing to Done Nov 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HUDI-2491] hoodie.datasource.hive_sync.mode=hms mode is supported in… #3799

[HUDI-2491] hoodie.datasource.hive_sync.mode=hms mode is supported in… #3799

fuyun2024 commented Oct 14, 2021 •

edited

fuyun2024 commented Oct 14, 2021

codope left a comment

codope Oct 14, 2021

codope Oct 14, 2021

codope Oct 14, 2021

fuyun2024 commented Oct 26, 2021

codope left a comment

fuyun2024 commented Oct 26, 2021

codope commented Oct 29, 2021

codope left a comment

fuyun2024 commented Nov 2, 2021

nsivabalan commented Nov 2, 2021

fuyun2024 commented Nov 2, 2021

codope commented Nov 3, 2021

codope commented Nov 3, 2021

fuyun2024 commented Nov 4, 2021

hudi-bot commented Nov 5, 2021

		HiveConf hiveConf = new HiveConf(conf, HiveConf.class);
		if (!DataSourceWriteOptions.METASTORE_URIS().defaultValue().equals(hiveSyncConfig.metastoreUris)) {

[HUDI-2491] hoodie.datasource.hive_sync.mode=hms mode is supported in… #3799

[HUDI-2491] hoodie.datasource.hive_sync.mode=hms mode is supported in… #3799

Conversation

fuyun2024 commented Oct 14, 2021 • edited

Tips

What is the purpose of the pull request

Brief change log

Verify this pull request

Committer checklist

fuyun2024 commented Oct 14, 2021

codope left a comment

Choose a reason for hiding this comment

codope Oct 14, 2021

Choose a reason for hiding this comment

codope Oct 14, 2021

Choose a reason for hiding this comment

codope Oct 14, 2021

Choose a reason for hiding this comment

fuyun2024 commented Oct 26, 2021

codope left a comment

Choose a reason for hiding this comment

fuyun2024 commented Oct 26, 2021

codope commented Oct 29, 2021

codope left a comment

Choose a reason for hiding this comment

fuyun2024 commented Nov 2, 2021

nsivabalan commented Nov 2, 2021

fuyun2024 commented Nov 2, 2021

codope commented Nov 3, 2021

codope commented Nov 3, 2021

fuyun2024 commented Nov 4, 2021

hudi-bot commented Nov 5, 2021

CI report:

fuyun2024 commented Oct 14, 2021 •

edited