xushiyan commented on a change in pull request #4175:
URL: https://github.com/apache/hudi/pull/4175#discussion_r824606045
##########
File path:
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestHiveSyncTool.java
##########
@@ -1115,31 +1036,40 @@ public void testTypeConverter(String syncMode) throws
Exception {
@ParameterizedTest
@MethodSource("syncMode")
public void testSyncWithoutDiffs(String syncMode) throws Exception {
- hiveSyncConfig.syncMode = syncMode;
- hiveSyncConfig.isConditionalSync = true;
- HiveTestUtil.hiveSyncConfig.batchSyncNum = 2;
- String tableName = HiveTestUtil.hiveSyncConfig.tableName +
HiveSyncTool.SUFFIX_SNAPSHOT_TABLE;
+ String tableName = HiveTestUtil.TABLE_NAME +
HiveSyncTool.SUFFIX_SNAPSHOT_TABLE;
+ hiveSyncProps.setProperty(HiveSyncConfig.HIVE_SYNC_MODE.key(), syncMode);
+ hiveSyncProps.setProperty(HiveSyncConfig.META_SYNC_CONDITIONAL_SYNC.key(),
"true");
String commitTime0 = "100";
String commitTime1 = "101";
String commitTime2 = "102";
HiveTestUtil.createMORTable(commitTime0, commitTime1, 2, true, true);
- HoodieHiveClient hiveClient =
- new HoodieHiveClient(HiveTestUtil.hiveSyncConfig,
HiveTestUtil.getHiveConf(), HiveTestUtil.fileSystem);
-
- HiveSyncTool tool = new HiveSyncTool(HiveTestUtil.hiveSyncConfig,
HiveTestUtil.getHiveConf(), HiveTestUtil.fileSystem);
- tool.syncHoodieTable();
+ reinitHiveSyncClient();
+ reSyncHiveTable();
assertTrue(hiveClient.doesTableExist(tableName));
assertEquals(commitTime1,
hiveClient.getLastCommitTimeSynced(tableName).get());
HiveTestUtil.addMORPartitions(0, true, true, true,
ZonedDateTime.now().plusDays(2), commitTime1, commitTime2);
- tool = new HiveSyncTool(HiveTestUtil.hiveSyncConfig,
HiveTestUtil.getHiveConf(), HiveTestUtil.fileSystem);
- tool.syncHoodieTable();
- hiveClient = new HoodieHiveClient(HiveTestUtil.hiveSyncConfig,
HiveTestUtil.getHiveConf(), HiveTestUtil.fileSystem);
+ reSyncHiveTable();
assertEquals(commitTime1,
hiveClient.getLastCommitTimeSynced(tableName).get());
}
+ private void reSyncHiveTable() {
+ hiveSyncTool.syncHoodieTable();
+ // we need renew the hiveclient after tool.syncHoodieTable(), because it
will close hive
+ // session, then lead to connection retry, we can see there is a exception
at log.
+ reinitHiveSyncClient();
+ }
+
+ private void reinitHiveSyncClient() {
+ hiveSyncTool = new HiveSyncTool(hiveSyncProps, HiveTestUtil.getHiveConf(),
fileSystem);
+ hiveClient = hiveSyncTool.hoodieHiveClient;
+ }
+
+ private int getPartitionFieldSize() {
+ return
hiveSyncProps.getString(HiveSyncConfig.META_SYNC_PARTITION_FIELDS.key()).split(",").length;
+ }
Review comment:
optional but i'd prefer to have static helpers for good portability.
##########
File path:
hudi-sync/hudi-dla-sync/src/main/java/org/apache/hudi/dla/DLASyncTool.java
##########
@@ -205,7 +206,8 @@ public static void main(String[] args) {
cmd.usage();
System.exit(1);
}
- FileSystem fs = FSUtils.getFs(cfg.basePath, new Configuration());
- new DLASyncTool(Utils.configToProperties(cfg), fs).syncHoodieTable();
+ Configuration hadoopConf = new Configuration();
+ FileSystem fs = FSUtils.getFs(cfg.basePath, hadoopConf);
+ new DLASyncTool(Utils.configToProperties(cfg), hadoopConf,
fs).syncHoodieTable();
Review comment:
`FSUtils.getFS()` alters hadoopConf internally and the caller has no
idea about it. This looks like passing an empty hadoopConf to it, though the
conf was updated. So suggest passing `fs.getConf()` to DLASyncTool to avoid the
confusion.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
