manojpec commented on a change in pull request #4406:
URL: https://github.com/apache/hudi/pull/4406#discussion_r774420533
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -460,7 +460,7 @@ protected void postCommit(HoodieTable<T, I, K, O> table,
HoodieCommitMetadata me
}
protected void runTableServicesInline(HoodieTable<T, I, K, O> table,
HoodieCommitMetadata metadata, Option<Map<String, String>> extraMetadata) {
- if (config.inlineTableServices()) {
+ if (config.isAnyTableServicesInline()) {
Review comment:
fixed
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -2221,11 +2236,17 @@ protected void setDefaults() {
HoodiePayloadConfig.newBuilder().fromProperties(writeConfig.getProps()).build());
writeConfig.setDefaultOnCondition(!isMetadataConfigSet,
HoodieMetadataConfig.newBuilder().withEngineType(engineType).fromProperties(writeConfig.getProps()).build());
- writeConfig.setDefaultOnCondition(!isLockConfigSet,
-
HoodieLockConfig.newBuilder().fromProperties(writeConfig.getProps()).build());
writeConfig.setDefaultOnCondition(!isPreCommitValidationConfigSet,
HoodiePreCommitValidatorConfig.newBuilder().fromProperties(writeConfig.getProps()).build());
writeConfig.setDefaultValue(TIMELINE_LAYOUT_VERSION_NUM,
String.valueOf(TimelineLayoutVersion.CURR_VERSION));
+
+ if (!isLockConfigSet) {
Review comment:
fixed.
##########
File path:
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/config/TestHoodieWriteConfig.java
##########
@@ -104,6 +112,91 @@ public void testDefaultMarkersTypeAccordingToEngineType() {
EngineType.JAVA, MarkerType.DIRECT));
}
+ @Test
+ public void testDefaultLockProviderWhenAsyncServicesEnabled() {
+ final String inProcessLockProviderClassName =
InProcessLockProvider.class.getCanonicalName();
+
+ // Any async clustering enabled should use InProcess lock provider
+ // as default when no other lock provider is set.
+
+ // 1. Async clustering
+ HoodieWriteConfig writeConfig = createWriteConfig(new HashMap<String,
String>() {
+ {
+ put(ASYNC_CLUSTERING_ENABLE.key(), "true");
+ put(INLINE_COMPACT.key(), "true");
+ put(AUTO_CLEAN.key(), "true");
+ put(ASYNC_CLEAN.key(), "false");
+ }
+ });
+ assertTrue(writeConfig.isAnyTableServicesAsync());
+ assertEquals(inProcessLockProviderClassName,
writeConfig.getLockProviderClass());
+
+ // 2. Async clean
+ writeConfig = createWriteConfig(new HashMap<String, String>() {
+ {
+ put(ASYNC_CLUSTERING_ENABLE.key(), "false");
+ put(INLINE_COMPACT.key(), "true");
+ put(AUTO_CLEAN.key(), "true");
+ put(ASYNC_CLEAN.key(), "true");
+ }
+ });
+ assertTrue(writeConfig.isAnyTableServicesAsync());
+ assertEquals(inProcessLockProviderClassName,
writeConfig.getLockProviderClass());
+
+ // 3. Async compaction
+ writeConfig = createWriteConfig(new HashMap<String, String>() {
+ {
+ put(ASYNC_CLUSTERING_ENABLE.key(), "false");
+ put(INLINE_COMPACT.key(), "false");
+ put(AUTO_CLEAN.key(), "true");
+ put(ASYNC_CLEAN.key(), "false");
+ }
+ });
+ assertTrue(writeConfig.isAnyTableServicesAsync());
+ assertEquals(inProcessLockProviderClassName,
writeConfig.getLockProviderClass());
+
+ // 4. All inline services
+ writeConfig = createWriteConfig(new HashMap<String, String>() {
+ {
+ put(ASYNC_CLUSTERING_ENABLE.key(), "false");
+ put(INLINE_COMPACT.key(), "true");
+ put(AUTO_CLEAN.key(), "true");
+ put(ASYNC_CLEAN.key(), "false");
+ }
+ });
+ assertFalse(writeConfig.isAnyTableServicesAsync());
+ assertTrue(writeConfig.isAnyTableServicesInline());
+ assertEquals(HoodieLockConfig.LOCK_PROVIDER_CLASS_NAME.defaultValue(),
writeConfig.getLockProviderClass());
+
+ // 5. User override for the lock provider should always take the precedence
+ writeConfig = HoodieWriteConfig.newBuilder()
+ .withPath("/tmp")
+ .withLockConfig(HoodieLockConfig.newBuilder()
+ .withLockProvider(FileSystemBasedLockProviderTestClass.class)
+ .build())
+ .build();
+ assertEquals(FileSystemBasedLockProviderTestClass.class.getName(),
writeConfig.getLockProviderClass());
+
+ // Default config should have default lock provider
+ writeConfig = createWriteConfig(Collections.emptyMap());
+ if (!writeConfig.isAnyTableServicesAsync()) {
+ assertEquals(HoodieLockConfig.LOCK_PROVIDER_CLASS_NAME.defaultValue(),
writeConfig.getLockProviderClass());
+ } else {
+ assertEquals(inProcessLockProviderClassName,
writeConfig.getLockProviderClass());
+ }
+ }
+
+ private HoodieWriteConfig createWriteConfig(Map<String, String> configs) {
+ final Properties properties = new Properties();
+ for (Map.Entry<String, String> entry : configs.entrySet()) {
Review comment:
fixed.
--
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]
