alexeykudinkin commented on code in PR #5269:
URL: https://github.com/apache/hudi/pull/5269#discussion_r1000040906
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/async/HoodieAsyncTableService.java:
##########
@@ -20,24 +20,37 @@
package org.apache.hudi.async;
import org.apache.hudi.client.RunsTableService;
+import org.apache.hudi.client.embedded.EmbeddedTimelineServerHelper;
+import org.apache.hudi.client.embedded.EmbeddedTimelineService;
+import org.apache.hudi.common.util.Option;
import org.apache.hudi.config.HoodieWriteConfig;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
public abstract class HoodieAsyncTableService extends HoodieAsyncService
implements RunsTableService {
+ protected final Object writeConfigUpdateLock = new Object();
protected HoodieWriteConfig writeConfig;
+ protected Option<EmbeddedTimelineService> embeddedTimelineService;
+ protected AtomicBoolean isWriteConfigUpdated = new AtomicBoolean(false);
protected HoodieAsyncTableService() {
}
- protected HoodieAsyncTableService(HoodieWriteConfig writeConfig) {
+ protected HoodieAsyncTableService(HoodieWriteConfig writeConfig,
Option<EmbeddedTimelineService> embeddedTimelineService) {
this.writeConfig = writeConfig;
+ this.embeddedTimelineService = embeddedTimelineService;
Review Comment:
@nsivabalan did you update this one? Don't see any changes here
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/async/HoodieAsyncTableService.java:
##########
@@ -47,4 +60,11 @@ public void start(Function<Boolean, Boolean>
onShutdownCallback) {
}
super.start(onShutdownCallback);
}
+
+ public void updateWriteConfig(HoodieWriteConfig writeConfig) {
+ synchronized (writeConfigUpdateLock) {
+ this.writeConfig =
EmbeddedTimelineServerHelper.updateWriteConfigWithTimelineServer(embeddedTimelineService.get(),
writeConfig);
+ isWriteConfigUpdated.set(true);
Review Comment:
@nsivabalan let's do the following instead:
- Let's keep the flag to trigger the refresh
- Let's make the flag more generic (like `shouldResetWriteClient`) so that
it's not coupled to writeConfig (there's no need for it to be)
- Let's reuse this logic b/w Clustering/Compaction services (see my comment
above)
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/async/HoodieAsyncTableService.java:
##########
@@ -47,4 +60,11 @@ public void start(Function<Boolean, Boolean>
onShutdownCallback) {
}
super.start(onShutdownCallback);
}
+
+ public void updateWriteConfig(HoodieWriteConfig writeConfig) {
+ synchronized (writeConfigUpdateLock) {
+ this.writeConfig =
EmbeddedTimelineServerHelper.updateWriteConfigWithTimelineServer(embeddedTimelineService.get(),
writeConfig);
+ isWriteConfigUpdated.set(true);
Review Comment:
Sync'd up offline: we can reset the write-client (it won't affect already
running compaction/clustering) but we won't be able to close existing client
synchronously, and will have to do an async follow-up anyway
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/async/AsyncCompactService.java:
##########
@@ -78,13 +81,26 @@ protected Pair<CompletableFuture, ExecutorService>
startService() {
while (!isShutdownRequested()) {
Review Comment:
This portion of the code is identical b/w Clustering/Compaction services.
Let's abstract it into a shared base class to avoid duplication.
--
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]
