From 641956f9639dda2604e07b14fe82d71e2023322f Mon Sep 17 00:00:00 2001 From: Raymond Xu <2701446+xushiyan@users.noreply.github.com> Date: Mon, 22 Nov 2021 18:38:01 -0800 Subject: [PATCH 1/2] [HUDI-2818] Fix 2to3 upgrade when set `hoodie.table.keygenerator.class` --- .../hudi/table/upgrade/OneToTwoUpgradeHandler.java | 4 ++-- .../hudi/table/upgrade/TwoToThreeUpgradeHandler.java | 12 +++++++++--- .../apache/hudi/table/upgrade/UpgradeDowngrade.java | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java index dddd5f4ac1410..efa0fe472c52c 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToTwoUpgradeHandler.java @@ -24,7 +24,7 @@ import org.apache.hudi.config.HoodieWriteConfig; import org.apache.hudi.keygen.constant.KeyGeneratorOptions; -import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; /** @@ -36,7 +36,7 @@ public class OneToTwoUpgradeHandler implements UpgradeHandler { public Map upgrade( HoodieWriteConfig config, HoodieEngineContext context, String instantTime, BaseUpgradeDowngradeHelper upgradeDowngradeHelper) { - Map tablePropsToAdd = new HashMap<>(); + Map tablePropsToAdd = new Hashtable<>(); tablePropsToAdd.put(HoodieTableConfig.PARTITION_FIELDS, upgradeDowngradeHelper.getPartitionColumns(config)); tablePropsToAdd.put(HoodieTableConfig.RECORDKEY_FIELDS, config.getString(KeyGeneratorOptions.RECORDKEY_FIELD_NAME.key())); tablePropsToAdd.put(HoodieTableConfig.BASE_FILE_FORMAT, config.getString(HoodieTableConfig.BASE_FILE_FORMAT)); diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java index e1dbfbbe2a51d..bff3788d56cfe 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TwoToThreeUpgradeHandler.java @@ -22,10 +22,12 @@ import org.apache.hudi.common.config.ConfigProperty; import org.apache.hudi.common.engine.HoodieEngineContext; import org.apache.hudi.common.table.HoodieTableConfig; +import org.apache.hudi.common.util.Option; +import org.apache.hudi.common.util.ValidationUtils; import org.apache.hudi.config.HoodieWriteConfig; import org.apache.hudi.metadata.HoodieTableMetadataUtil; -import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; /** @@ -40,10 +42,14 @@ public Map upgrade(HoodieWriteConfig config, HoodieEngin // table has been updated and is not backward compatible. HoodieTableMetadataUtil.deleteMetadataTable(config.getBasePath(), context); } - Map tablePropsToAdd = new HashMap<>(); + Map tablePropsToAdd = new Hashtable<>(); tablePropsToAdd.put(HoodieTableConfig.URL_ENCODE_PARTITIONING, config.getStringOrDefault(HoodieTableConfig.URL_ENCODE_PARTITIONING)); tablePropsToAdd.put(HoodieTableConfig.HIVE_STYLE_PARTITIONING_ENABLE, config.getStringOrDefault(HoodieTableConfig.HIVE_STYLE_PARTITIONING_ENABLE)); - tablePropsToAdd.put(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME, config.getString(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME)); + String keyGenClassName = Option.ofNullable(config.getString(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME)) + .orElse(config.getString(HoodieWriteConfig.KEYGENERATOR_CLASS_NAME)); + ValidationUtils.checkState(keyGenClassName != null, String.format("Missing config: %s or %s", + HoodieTableConfig.KEY_GENERATOR_CLASS_NAME, HoodieWriteConfig.KEYGENERATOR_CLASS_NAME)); + tablePropsToAdd.put(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME, keyGenClassName); return tablePropsToAdd; } } diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java index c1b891095af5c..0e8f752a8f682 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java @@ -31,7 +31,7 @@ import org.apache.log4j.LogManager; import org.apache.log4j.Logger; -import java.util.HashMap; +import java.util.Hashtable; import java.util.Map; /** @@ -110,7 +110,7 @@ public void run(HoodieTableVersion toVersion, String instantTime) { // Perform the actual upgrade/downgrade; this has to be idempotent, for now. LOG.info("Attempting to move table from version " + fromVersion + " to " + toVersion); - Map tableProps = new HashMap<>(); + Map tableProps = new Hashtable<>(); if (fromVersion.versionCode() < toVersion.versionCode()) { // upgrade while (fromVersion.versionCode() < toVersion.versionCode()) { From 6d40cfb4ea2fe7be8da3df926018a847579f6b33 Mon Sep 17 00:00:00 2001 From: Raymond Xu <2701446+xushiyan@users.noreply.github.com> Date: Tue, 23 Nov 2021 17:04:07 -0800 Subject: [PATCH 2/2] add UT --- .../upgrade/TestTwoToThreeUpgradeHandler.java | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java new file mode 100644 index 0000000000000..35928dc7cf319 --- /dev/null +++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTwoToThreeUpgradeHandler.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hudi.table.upgrade; + +import org.apache.hudi.common.config.ConfigProperty; +import org.apache.hudi.common.config.HoodieMetadataConfig; +import org.apache.hudi.common.table.HoodieTableConfig; +import org.apache.hudi.config.HoodieWriteConfig; +import org.apache.hudi.keygen.KeyGenerator; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class TestTwoToThreeUpgradeHandler { + + HoodieWriteConfig config; + + @BeforeEach + void setUp() { + config = HoodieWriteConfig.newBuilder() + .forTable("foo") + .withPath("/foo") + .withMetadataConfig(HoodieMetadataConfig.newBuilder().enable(false).build()) + .build(); + } + + @ParameterizedTest + @ValueSource(strings = {"hoodie.table.keygenerator.class", "hoodie.datasource.write.keygenerator.class"}) + void upgradeHandlerShouldRetrieveKeyGeneratorConfig(String keyGenConfigKey) { + config.setValue(keyGenConfigKey, KeyGenerator.class.getName()); + TwoToThreeUpgradeHandler handler = new TwoToThreeUpgradeHandler(); + Map kv = handler.upgrade(config, null, null, null); + assertEquals(KeyGenerator.class.getName(), kv.get(HoodieTableConfig.KEY_GENERATOR_CLASS_NAME)); + } + + @Test + void upgradeHandlerShouldThrowWhenKeyGeneratorNotSet() { + TwoToThreeUpgradeHandler handler = new TwoToThreeUpgradeHandler(); + Throwable t = assertThrows(IllegalStateException.class, () -> handler + .upgrade(config, null, null, null)); + assertTrue(t.getMessage().startsWith("Missing config:")); + } +}