[HUDI-1534]HiveSyncTool-It is not necessary to use JDBC and MetaStoreClient at the same time by vinnielhj · Pull Request #2532 · apache/hudi

vinnielhj · 2021-02-04T11:05:19Z

To synchronize the hudi meta information to the Hive metastore, it is not necessary to use both JDBC and metastoreClient. Now modified some methods, unified use metastoreClient to synchronize, mainly the createTable, updateTableDefinition, addPartitionsToTable and updatePartitionsToTable methods of the HoodieHiveClient class. At the same time, a new class DLASchemaUtil was created to isolate the synchronization of DLA tables.
please review tihs pull, thanks.

codecov-io · 2021-02-04T15:54:14Z

Codecov Report

Merging #2532 (afd7303) into master (c30481f) will decrease coverage by 30.43%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master    #2532       +/-   ##
=============================================
- Coverage     50.90%   20.47%   -30.44%     
+ Complexity     3167      101     -3066     
=============================================
  Files           433       53      -380     
  Lines         19806     1929    -17877     
  Branches       2032      230     -1802     
=============================================
- Hits          10083      395     -9688     
+ Misses         8904     1516     -7388     
+ Partials        819       18      -801

Flag	Coverage Δ	Complexity Δ
hudicli	`?`	`?`
hudiclient	`?`	`?`
hudicommon	`?`	`?`
hudiflink	`?`	`?`
hudihadoopmr	`?`	`?`
hudisparkdatasource	`?`	`?`
hudisync	`?`	`?`
huditimelineservice	`?`	`?`
hudiutilities	`20.47% <ø> (-48.99%)`	`0.00 <ø> (ø)`

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

Impacted Files	Coverage Δ	Complexity Δ
...va/org/apache/hudi/utilities/IdentitySplitter.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-2.00%)`
...va/org/apache/hudi/utilities/schema/SchemaSet.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-3.00%)`
...a/org/apache/hudi/utilities/sources/RowSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-4.00%)`
.../org/apache/hudi/utilities/sources/AvroSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-1.00%)`
.../org/apache/hudi/utilities/sources/JsonSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-1.00%)`
...rg/apache/hudi/utilities/sources/CsvDFSSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-10.00%)`
...g/apache/hudi/utilities/sources/JsonDFSSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-4.00%)`
...apache/hudi/utilities/sources/JsonKafkaSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-6.00%)`
...pache/hudi/utilities/sources/ParquetDFSSource.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-5.00%)`
...lities/schema/SchemaProviderWithPostProcessor.java	`0.00% <0.00%> (-100.00%)`	`0.00% <0.00%> (-4.00%)`
... and 397 more

n3nash · 2021-02-04T19:13:07Z

@satishkotha can you help review ?

…1534 � Conflicts: � hudi-sync/hudi-dla-sync/src/main/java/org/apache/hudi/dla/DLASyncTool.java � hudi-sync/hudi-dla-sync/src/main/java/org/apache/hudi/dla/util/DLASchemaUtil.java � hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveClient.java � hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java

vinnielhj · 2021-02-11T03:16:56Z

hi @satishkotha ,please review,thank you very much

satishkotha

Sorry for delay in reviewing. I was away from work for last week.

satishkotha · 2021-02-18T22:42:23Z

hudi-sync/hudi-dla-sync/src/main/java/org/apache/hudi/dla/util/DLASchemaUtil.java

+import java.util.Set;
+
+/**
+ * Schema Utilities.


This looks identical to HiveSchemaUtil. Can you help me understand why we need this separate util class? Is 'tickSupport' only the difference? Do you think we can pass that as a flag/config to avoid code duplication?

In the master branch, the realization of DLA table synchronization reuses many methods of the HiveSchemaUtil class, such as: createHiveStruct, convertField, etc. This change only modifies the synchronization of Hive and does not involve DLA. So the DLASchemaUtil class is added for DLA table synchronization. (It is actually the former HiveSchemaUtil.java).

@satishkotha Sorry, in the branch I submitted before, DLASyncTool.java still uses HiveSchemaUtil. This is my mistake. I modified this to use DLASchemaUtil and resubmitted the code.

satishkotha · 2021-02-18T22:52:34Z

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

-    this.configuration = configuration;
-    // Support both JDBC and metastore based implementations for backwards compatiblity. Future users should
-    // disable jdbc and depend on metastore client for all hive registrations
-    if (cfg.useJdbc) {


Seems like this is good to have backward compatibility and can be disabled using config easily (we can also set it to false by default?)

Do you think this adds a lot of overhead? Is this change discussed in open source meetup/email lists?

For Hive synchronization, any JDBC-related code and configuration have not been used after this change, so this content is removed from HoodieHiveClient.java. Welcome to continue the discussion @satishkotha

satishkotha · 2021-02-18T22:55:36Z

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

-      }
-    } else {
-      updateHiveSQLUsingHiveDriver(s);
+  public void createDataBase(String databaseName, String location, String description) {


nit: Database instead of DataBase everywhere?

Yes, this is a problem. I have changed the spelling of Database used by Hive synchronization to Database (the second b is lowercase) and resubmit the code.

satishkotha · 2021-02-18T23:02:21Z

hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestHiveSyncTool.java


    String schemaString = HiveSchemaUtil.generateSchemaString(schema);
-    assertEquals("`int_list` ARRAY< int>", schemaString);
+    assertEquals("int_list ARRAY<int>", schemaString);


Can you add test coverage for DLASchemaUtil#generateSchemaString (and other methods in DLASchemaUtil?)

Now I have added the TestDLASyncTool class to test the methods of the DLASchemaUtil class.please review.thanks.

… use DLASchemaUtil.

satishkotha · 2021-02-23T01:28:56Z

@lhjzmn Can you start a discuss thread in dev and users channel. I think removing JDBC support requires consensus in community

rubenssoto · 2021-02-24T04:19:12Z

Hello People,

Is this PR could help in my problem?

#2588

@satishkotha @lhjzmn

satishkotha · 2021-02-24T18:59:02Z

Hello People,

Is this PR could help in my problem?

#2588

@satishkotha @lhjzmn

@rubenssoto I dont think so. This PR is removing jdbc support to connect to Hive. Hudi already supports a way to disable jdbc (See https://hudi.apache.org/docs/configurations.html#HIVE_USE_JDBC_OPT_KEY). I dont know how you are connecting, if your hive version, supports thrift metastore interface, you can try disabling jdbc and see if that helps.

satishkotha · 2021-03-27T00:38:48Z

@lhjzmn we discussed this in one of the OSS meetings. we want to keep jdbc support for backward compatibility reasons. we can separate out MetastoreClient as a separate class though. please let me know what you think.

jsbali · 2021-04-22T16:35:04Z

Hey @lhjzmn If you are not working on it. I can take it up and build on top your code.
One way we can do it is support all three mode ie via

JDBC
HiveDriver
HMS

Or we can get rid of HiveDriver code and replace it with HMS calls and provide only two options like

useJDBC
useHMS

Or if we are sure that jdbc is not required we can completely get rid of it as well which is what this PR does.
@n3nash @vinothchandar @satishkotha What do you guys think?

vinothchandar · 2021-09-07T21:31:46Z

@jsbali Love to see this home. Do you want to grab since wokr if you are still interested?

jsbali · 2021-09-13T10:29:31Z

@vinothchandar This PR was about removing all support for jdbc. hiveql and supporting only HMS. We went ahead with a different approach in #2879 and added support for HMS along with jdbc and hiveql.
This PR can be closed.

hudi-bot · 2021-11-05T02:52:11Z

CI report:

bcf5f75 UNKNOWN

Bot commands

@hudi-bot supports the following commands:

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

nsivabalan · 2021-12-13T14:42:43Z

closing this PR as we have added support for hive sync modes and for hms, users don't have to supply any additional configs like one has to provide for jdbc way.

hongjie.li added 6 commits February 1, 2021 10:28

[HUDI-1534]always use metaStoreClient

792a25d

Merge branch 'master' of https://github.com/apache/hudi

f598b01

[HUDI-1534]Add log printing

3bd625d

[HUDI-1534]fix add partition bug

bf5dac6

[HUDI-1534]add log

66bd255

[HUDI-1534] for test

d2bccb0

n3nash requested a review from satishkotha February 4, 2021 19:12

n3nash assigned satishkotha Feb 4, 2021

hongjie.li added 16 commits February 5, 2021 15:32

[HUDI-1534] for test

b40132e

Merge branch 'master' of https://github.com/apache/hudi into HUDI-1534

be81fce

[HUDI-1534] for test

732fdb3

[HUDI-1534]for test

67fb6d6

[HUDI-1534]fix struct type

f560507

[HUDI-1534]fix map type

9aa1fab

[HUDI-1534]fix create db bug

b408d7c

Merge branch 'master' of https://github.com/apache/hudi into HUDI-1534

bc2a500

[HUDI-1534]fix create db bug

8e84276

Merge branch 'master' of https://github.com/apache/hudi into HUDI-1534

c85619a

[HUDI-1534]fix updatePartitionToTable bug

d8c4626

[HUDI-1534]fix updatePartitionToTable bug

242938a

[HUDI-1534]fix int_list bug

d40c68d

[HUDI-1534]fix int_list bug

fce9e44

[HUDI-1534]add log print

8c5958c

nsivabalan added the priority:high Significant impact; potential bugs label Feb 11, 2021

satishkotha reviewed Feb 18, 2021

View reviewed changes

[HUDI-1534]DLASyncTool.java should not use HiveSchemaUtil, but should…

afd7303

… use DLASchemaUtil.

[HUDI-1534]Add unit test of DLASchemaUtil class

bcf5f75

vinothchandar mentioned this pull request May 11, 2021

[HUDI-1848] Adding support for HMS for running DDL queries in hive-sy… #2879

Merged

5 tasks

vinothchandar assigned vinothchandar and unassigned satishkotha Sep 7, 2021

nsivabalan closed this Dec 13, 2021

Conversation

vinnielhj commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

n3nash commented Feb 4, 2021

Uh oh!

vinnielhj commented Feb 11, 2021

Uh oh!

satishkotha left a comment

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!

Choose a reason for hiding this comment

Uh oh!

satishkotha commented Feb 23, 2021

Uh oh!

rubenssoto commented Feb 24, 2021

Uh oh!

satishkotha commented Feb 24, 2021

Uh oh!

satishkotha commented Mar 27, 2021

Uh oh!

jsbali commented Apr 22, 2021

Uh oh!

vinothchandar commented Sep 7, 2021

Uh oh!

jsbali commented Sep 13, 2021

Uh oh!

hudi-bot commented Nov 5, 2021

CI report:

Uh oh!

nsivabalan commented Dec 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

vinnielhj commented Feb 4, 2021 •

edited

Loading

codecov-io commented Feb 4, 2021 •

edited

Loading