From b10b162c6aa0f46465eb77dd48ce316f49266745 Mon Sep 17 00:00:00 2001 From: xicm Date: Tue, 12 Apr 2022 18:01:01 +0800 Subject: [PATCH 1/5] HUDI-3862 Some default configurations in HoodieHBaseIndexConfig do not work --- .../apache/hudi/config/HoodieWriteConfig.java | 22 +++++++++---------- .../hudi/common/config/HoodieConfig.java | 9 ++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java index d861ffe970c80..6f4bb151e0a05 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java @@ -1453,15 +1453,15 @@ public String getHbaseTableName() { } public int getHbaseIndexGetBatchSize() { - return getInt(HoodieHBaseIndexConfig.GET_BATCH_SIZE); + return getIntOrDefault(HoodieHBaseIndexConfig.GET_BATCH_SIZE); } public Boolean getHBaseIndexRollbackSync() { - return getBoolean(HoodieHBaseIndexConfig.ROLLBACK_SYNC_ENABLE); + return getBooleanOrDefault(HoodieHBaseIndexConfig.ROLLBACK_SYNC_ENABLE); } public int getHbaseIndexPutBatchSize() { - return getInt(HoodieHBaseIndexConfig.PUT_BATCH_SIZE); + return getIntOrDefault(HoodieHBaseIndexConfig.PUT_BATCH_SIZE); } public boolean getHbaseIndexPutBatchSizeAutoCompute() { @@ -1469,27 +1469,27 @@ public boolean getHbaseIndexPutBatchSizeAutoCompute() { } public String getHBaseQPSResourceAllocatorClass() { - return getString(HoodieHBaseIndexConfig.QPS_ALLOCATOR_CLASS_NAME); + return getStringOrDefault(HoodieHBaseIndexConfig.QPS_ALLOCATOR_CLASS_NAME); } public String getHBaseQPSZKnodePath() { - return getString(HoodieHBaseIndexConfig.ZKPATH_QPS_ROOT); + return getStringOrDefault(HoodieHBaseIndexConfig.ZKPATH_QPS_ROOT); } public String getHBaseZkZnodeSessionTimeout() { - return getString(HoodieHBaseIndexConfig.ZK_SESSION_TIMEOUT_MS); + return getStringOrDefault(HoodieHBaseIndexConfig.ZK_SESSION_TIMEOUT_MS); } public String getHBaseZkZnodeConnectionTimeout() { - return getString(HoodieHBaseIndexConfig.ZK_CONNECTION_TIMEOUT_MS); + return getStringOrDefault(HoodieHBaseIndexConfig.ZK_CONNECTION_TIMEOUT_MS); } public boolean getHBaseIndexShouldComputeQPSDynamically() { - return getBoolean(HoodieHBaseIndexConfig.COMPUTE_QPS_DYNAMICALLY); + return getBooleanOrDefault(HoodieHBaseIndexConfig.COMPUTE_QPS_DYNAMICALLY); } public int getHBaseIndexDesiredPutsTime() { - return getInt(HoodieHBaseIndexConfig.DESIRED_PUTS_TIME_IN_SECONDS); + return getIntOrDefault(HoodieHBaseIndexConfig.DESIRED_PUTS_TIME_IN_SECONDS); } public String getBloomFilterType() { @@ -1506,7 +1506,7 @@ public int getDynamicBloomFilterMaxNumEntries() { * the jobs would be (0.17) 1/6, 0.33 (2/6) and 0.5 (3/6) respectively. */ public float getHbaseIndexQPSFraction() { - return getFloat(HoodieHBaseIndexConfig.QPS_FRACTION); + return getFloatOrDefault(HoodieHBaseIndexConfig.QPS_FRACTION); } public float getHBaseIndexMinQPSFraction() { @@ -1522,7 +1522,7 @@ public float getHBaseIndexMaxQPSFraction() { * Hoodie jobs to an Hbase Region Server */ public int getHbaseIndexMaxQPSPerRegionServer() { - return getInt(HoodieHBaseIndexConfig.MAX_QPS_PER_REGION_SERVER); + return getIntOrDefault(HoodieHBaseIndexConfig.MAX_QPS_PER_REGION_SERVER); } public boolean getHbaseIndexUpdatePartitionPath() { diff --git a/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java b/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java index c77e292b4775f..b1edd57b34370 100644 --- a/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java +++ b/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java @@ -171,6 +171,15 @@ public Float getFloat(ConfigProperty configProperty) { return rawValue.map(v -> Float.parseFloat(v.toString())).orElse(null); } + public Float getFloatOrDefault(ConfigProperty configProperty) { + return getFloatOrDefault(configProperty, configProperty.defaultValue()); + } + + public Float getFloatOrDefault(ConfigProperty configProperty, T defaultVal) { + Option rawValue = getRawValue(configProperty); + return rawValue.map(v -> Float.parseFloat(v.toString())).orElse((float)defaultVal); + } + public Double getDouble(ConfigProperty configProperty) { Option rawValue = getRawValue(configProperty); return rawValue.map(v -> Double.parseDouble(v.toString())).orElse(null); From 4833349933c202632b3230f3db85ed3dac3a47cc Mon Sep 17 00:00:00 2001 From: xicm Date: Wed, 13 Apr 2022 10:05:22 +0800 Subject: [PATCH 2/5] modify getFloatOrDefault --- .../java/org/apache/hudi/common/config/HoodieConfig.java | 2 +- .../org/apache/hudi/common/config/TestConfigProperty.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java b/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java index b1edd57b34370..3b69d7b01c708 100644 --- a/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java +++ b/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java @@ -177,7 +177,7 @@ public Float getFloatOrDefault(ConfigProperty configProperty) { public Float getFloatOrDefault(ConfigProperty configProperty, T defaultVal) { Option rawValue = getRawValue(configProperty); - return rawValue.map(v -> Float.parseFloat(v.toString())).orElse((float)defaultVal); + return rawValue.map(v -> Float.parseFloat(v.toString())).orElse((Float)defaultVal); } public Double getDouble(ConfigProperty configProperty) { diff --git a/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java b/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java index 6cbb9bd48e496..888caedbc0df2 100644 --- a/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java +++ b/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java @@ -56,6 +56,11 @@ public class TestConfigProperty extends HoodieConfig { }) .withDocumentation("Fake config only for testing"); + public static ConfigProperty FAKE_FLOAT_CONFIG = ConfigProperty + .key("test.fake.float.config") + .defaultValue(0.5f) + .withDocumentation("Fake config only for testing"); + @Test public void testGetTypedValue() { HoodieConfig hoodieConfig = new HoodieConfig(); @@ -87,6 +92,7 @@ public void testGetOrDefault() { HoodieConfig hoodieConfig = new HoodieConfig(props); assertEquals("1", hoodieConfig.getStringOrDefault(FAKE_STRING_CONFIG)); assertEquals("2", hoodieConfig.getStringOrDefault(FAKE_STRING_CONFIG, "2")); + assertEquals(0.5f, hoodieConfig.getFloatOrDefault(FAKE_FLOAT_CONFIG)); } @Test From 192c99c64209bfd34220c0ef6c9acd47926f2117 Mon Sep 17 00:00:00 2001 From: xicm Date: Wed, 13 Apr 2022 12:37:18 +0800 Subject: [PATCH 3/5] fix ut errors --- .../java/org/apache/hudi/common/config/TestConfigProperty.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java b/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java index 888caedbc0df2..dc09e8362dbfd 100644 --- a/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java +++ b/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java @@ -119,6 +119,6 @@ public void testInference() { @Test public void testSetDefaults() { setDefaults(this.getClass().getName()); - assertEquals(3, getProps().size()); + assertEquals(4, getProps().size()); } } From 6ebdccd70df7ed28e86f2a5737932770824c122f Mon Sep 17 00:00:00 2001 From: xicm Date: Wed, 13 Apr 2022 15:39:31 +0800 Subject: [PATCH 4/5] set default values of HoodieHBaseIndexConfig --- .../src/main/java/org/apache/hudi/config/HoodieWriteConfig.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java index 6f4bb151e0a05..b0839cf76f506 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java @@ -2465,6 +2465,8 @@ protected void setDefaults() { writeConfig.setDefaultValue(MARKERS_TYPE, getDefaultMarkersType(engineType)); // Check for mandatory properties writeConfig.setDefaults(HoodieWriteConfig.class.getName()); + // Set default values of HoodieHBaseIndexConfig + writeConfig.setDefaults(HoodieHBaseIndexConfig.class.getName()); // Make sure the props is propagated writeConfig.setDefaultOnCondition( !isIndexConfigSet, HoodieIndexConfig.newBuilder().withEngineType(engineType).fromProperties( From 4082b4566643bf3a37ae7b924182e96e2057cb81 Mon Sep 17 00:00:00 2001 From: xicm Date: Wed, 13 Apr 2022 16:09:48 +0800 Subject: [PATCH 5/5] revert --- .../apache/hudi/config/HoodieWriteConfig.java | 22 +++++++++---------- .../hudi/common/config/HoodieConfig.java | 9 -------- .../common/config/TestConfigProperty.java | 8 +------ 3 files changed, 12 insertions(+), 27 deletions(-) diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java index b0839cf76f506..42728757ad83e 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java @@ -1453,15 +1453,15 @@ public String getHbaseTableName() { } public int getHbaseIndexGetBatchSize() { - return getIntOrDefault(HoodieHBaseIndexConfig.GET_BATCH_SIZE); + return getInt(HoodieHBaseIndexConfig.GET_BATCH_SIZE); } public Boolean getHBaseIndexRollbackSync() { - return getBooleanOrDefault(HoodieHBaseIndexConfig.ROLLBACK_SYNC_ENABLE); + return getBoolean(HoodieHBaseIndexConfig.ROLLBACK_SYNC_ENABLE); } public int getHbaseIndexPutBatchSize() { - return getIntOrDefault(HoodieHBaseIndexConfig.PUT_BATCH_SIZE); + return getInt(HoodieHBaseIndexConfig.PUT_BATCH_SIZE); } public boolean getHbaseIndexPutBatchSizeAutoCompute() { @@ -1469,27 +1469,27 @@ public boolean getHbaseIndexPutBatchSizeAutoCompute() { } public String getHBaseQPSResourceAllocatorClass() { - return getStringOrDefault(HoodieHBaseIndexConfig.QPS_ALLOCATOR_CLASS_NAME); + return getString(HoodieHBaseIndexConfig.QPS_ALLOCATOR_CLASS_NAME); } public String getHBaseQPSZKnodePath() { - return getStringOrDefault(HoodieHBaseIndexConfig.ZKPATH_QPS_ROOT); + return getString(HoodieHBaseIndexConfig.ZKPATH_QPS_ROOT); } public String getHBaseZkZnodeSessionTimeout() { - return getStringOrDefault(HoodieHBaseIndexConfig.ZK_SESSION_TIMEOUT_MS); + return getString(HoodieHBaseIndexConfig.ZK_SESSION_TIMEOUT_MS); } public String getHBaseZkZnodeConnectionTimeout() { - return getStringOrDefault(HoodieHBaseIndexConfig.ZK_CONNECTION_TIMEOUT_MS); + return getString(HoodieHBaseIndexConfig.ZK_CONNECTION_TIMEOUT_MS); } public boolean getHBaseIndexShouldComputeQPSDynamically() { - return getBooleanOrDefault(HoodieHBaseIndexConfig.COMPUTE_QPS_DYNAMICALLY); + return getBoolean(HoodieHBaseIndexConfig.COMPUTE_QPS_DYNAMICALLY); } public int getHBaseIndexDesiredPutsTime() { - return getIntOrDefault(HoodieHBaseIndexConfig.DESIRED_PUTS_TIME_IN_SECONDS); + return getInt(HoodieHBaseIndexConfig.DESIRED_PUTS_TIME_IN_SECONDS); } public String getBloomFilterType() { @@ -1506,7 +1506,7 @@ public int getDynamicBloomFilterMaxNumEntries() { * the jobs would be (0.17) 1/6, 0.33 (2/6) and 0.5 (3/6) respectively. */ public float getHbaseIndexQPSFraction() { - return getFloatOrDefault(HoodieHBaseIndexConfig.QPS_FRACTION); + return getFloat(HoodieHBaseIndexConfig.QPS_FRACTION); } public float getHBaseIndexMinQPSFraction() { @@ -1522,7 +1522,7 @@ public float getHBaseIndexMaxQPSFraction() { * Hoodie jobs to an Hbase Region Server */ public int getHbaseIndexMaxQPSPerRegionServer() { - return getIntOrDefault(HoodieHBaseIndexConfig.MAX_QPS_PER_REGION_SERVER); + return getInt(HoodieHBaseIndexConfig.MAX_QPS_PER_REGION_SERVER); } public boolean getHbaseIndexUpdatePartitionPath() { diff --git a/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java b/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java index 3b69d7b01c708..c77e292b4775f 100644 --- a/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java +++ b/hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java @@ -171,15 +171,6 @@ public Float getFloat(ConfigProperty configProperty) { return rawValue.map(v -> Float.parseFloat(v.toString())).orElse(null); } - public Float getFloatOrDefault(ConfigProperty configProperty) { - return getFloatOrDefault(configProperty, configProperty.defaultValue()); - } - - public Float getFloatOrDefault(ConfigProperty configProperty, T defaultVal) { - Option rawValue = getRawValue(configProperty); - return rawValue.map(v -> Float.parseFloat(v.toString())).orElse((Float)defaultVal); - } - public Double getDouble(ConfigProperty configProperty) { Option rawValue = getRawValue(configProperty); return rawValue.map(v -> Double.parseDouble(v.toString())).orElse(null); diff --git a/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java b/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java index dc09e8362dbfd..6cbb9bd48e496 100644 --- a/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java +++ b/hudi-common/src/test/java/org/apache/hudi/common/config/TestConfigProperty.java @@ -56,11 +56,6 @@ public class TestConfigProperty extends HoodieConfig { }) .withDocumentation("Fake config only for testing"); - public static ConfigProperty FAKE_FLOAT_CONFIG = ConfigProperty - .key("test.fake.float.config") - .defaultValue(0.5f) - .withDocumentation("Fake config only for testing"); - @Test public void testGetTypedValue() { HoodieConfig hoodieConfig = new HoodieConfig(); @@ -92,7 +87,6 @@ public void testGetOrDefault() { HoodieConfig hoodieConfig = new HoodieConfig(props); assertEquals("1", hoodieConfig.getStringOrDefault(FAKE_STRING_CONFIG)); assertEquals("2", hoodieConfig.getStringOrDefault(FAKE_STRING_CONFIG, "2")); - assertEquals(0.5f, hoodieConfig.getFloatOrDefault(FAKE_FLOAT_CONFIG)); } @Test @@ -119,6 +113,6 @@ public void testInference() { @Test public void testSetDefaults() { setDefaults(this.getClass().getName()); - assertEquals(4, getProps().size()); + assertEquals(3, getProps().size()); } }