You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "the-other-tim-brown (via GitHub)" <gi...@apache.org> on 2023/11/16 18:59:12 UTC
[PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
the-other-tim-brown opened a new pull request, #10122:
URL: https://github.com/apache/hudi/pull/10122
### Change Logs
- Allows users to reuse a running timeline server for other tables
- Minor performance improvements around object reuse and creation
- Use of AfterburnerModule where we saw a 22% increase in throughput for to/from json on the base file DTOs used in the timeline server
### Impact
- Allows users to lower overhead when running multiple writers in the same JVM by reusing the underlying server instance
- Performance improvement in the json (de)serialization
### Risk level (write none, low medium or high below)
low - feature for reusing the server is off by default
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change_
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1400959986
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +214,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
Review Comment:
I thought about this and also request handlers that tim pointed out. Here is my take on this.
By default we are going to disable reuse. So, users should enable only on a need basis. I will add more documentation to call out that we expect some homoegenous set up to use this.
For eg, in multi-table deltastreamer, if user has 10 tables, all 10 of them is expected to have similar config values for other secondary configs like num threads w/ timeline server, marker batch num threads, marker parallelism etc. If the 10 tables are going to have slightly diff values for these secondary configs, its better for user to not use this feature only.
As of now, the key properties that will determine whether one can reuse an existing timeline server or not is
- hostAddr
- metadata enable flag
- early conflict detection enabled
- marker type
- isTimelineServerBasedInstantStateEnabled
if these matches, hudi will reuse an existing timeline server. So the first one to instantiate the timeline server, the resp writeConfig will be applied.
We can revisit this for key properties going forward. but may not be scalable/maintainable if we account for every minor config difference.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815213231
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b10a5699df7578ad8c6c6239bb887f50b6438e34 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399815363
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
+ private static final Map<Integer, EmbeddedTimelineService> RUNNING_SERVICES = new HashMap<>();
+ private static final Registry METRICS_REGISTRY = Registry.getRegistry("TimelineService");
+ private static final String NUM_EMBEDDED_TIMELINE_SERVERS = "numEmbeddedTimelineServers";
private int serverPort;
private String hostAddr;
- private HoodieEngineContext context;
+ private final HoodieEngineContext context;
private final SerializableConfiguration hadoopConf;
private final HoodieWriteConfig writeConfig;
- private final String basePath;
+ private TimelineService.Config serviceConfig;
+ private final Set<String> basePaths; // the set of base paths using this EmbeddedTimelineService
private transient FileSystemViewManager viewManager;
private transient TimelineService server;
- public EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
+ private EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
setHostAddr(embeddedTimelineServiceHostAddr);
this.context = context;
this.writeConfig = writeConfig;
- this.basePath = writeConfig.getBasePath();
+ this.basePaths = new HashSet<>();
+ this.basePaths.add(writeConfig.getBasePath());
this.hadoopConf = context.getHadoopConf();
this.viewManager = createViewManager();
}
+ /**
+ * Returns an existing embedded timeline service if one is running for the given configuration and reuse is enabled, or starts a new one.
+ * @param context The {@link HoodieEngineContext} for the client
+ * @param embeddedTimelineServiceHostAddr The host address to use for the service (nullable)
+ * @param writeConfig The {@link HoodieWriteConfig} for the client
+ * @return A running {@link EmbeddedTimelineService}
+ * @throws IOException if an error occurs while starting the service
+ */
+ public static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) throws IOException {
+ return getOrStartEmbeddedTimelineService(context, embeddedTimelineServiceHostAddr, writeConfig, TimelineService::new);
+ }
+
+ static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig,
+ TimelineServiceCreator timelineServiceCreator) throws IOException {
+ // if reuse is enabled, check if any existing instances are compatible
+ if (writeConfig.isEmbeddedTimelineServerReuseEnabled()) {
+ synchronized (SERVICE_LOCK) {
+ for (EmbeddedTimelineService service : RUNNING_SERVICES.values()) {
+ if (service.canReuseFor(writeConfig, embeddedTimelineServiceHostAddr)) {
+ service.addBasePath(writeConfig.getBasePath());
+ LOG.info("Reusing existing embedded timeline server with configuration: " + service.serviceConfig);
+ return service;
+ }
+ }
+ // if no compatible instance is found, create a new one
+ EmbeddedTimelineService service = createAndStartService(context, embeddedTimelineServiceHostAddr, writeConfig, timelineServiceCreator);
+ RUNNING_SERVICES.put(service.serverPort, service);
Review Comment:
Should we also track which handlers are enabled? I see we only add some request handlers if a flag is enabled
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398063189
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +196,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
+ }
+
+ private boolean metadataConfigsAreEquivalent(Properties properties1, Properties properties2) {
+ Set<Object> metadataConfigs = new HashSet<>(properties1.keySet());
Review Comment:
Can do a shortcut check for whether the reference equals: `props1 == props2`.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398306501
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
The timeline server reuse for Flink was previously enabled by default, I try to understand what the discrepencies are introduced in Flink side.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1823515572
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
}, {
"hash" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096",
"triggerID" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"triggerType" : "PUSH"
}, {
"hash" : "cae921ac9d016d28b87139b5c0fd24debadf1592",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21109",
"triggerID" : "cae921ac9d016d28b87139b5c0fd24debadf1592",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 597f6d7bd7134d635ad5a675bd398ba03faafef8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096)
* cae921ac9d016d28b87139b5c0fd24debadf1592 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21109)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1822849025
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
}, {
"hash" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096",
"triggerID" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 597f6d7bd7134d635ad5a675bd398ba03faafef8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1820002829
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965)
* c78d9570c87afce7a27725b37a3ebb77913199b7 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815225533
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b10a5699df7578ad8c6c6239bb887f50b6438e34 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956)
* 1877ef99bb4c939181b5341e19c44cbba742d7cd UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399943850
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
Here the `pipeline1` and `pipeline2` share the same table path right? I'm curious why the exception throws with this patch.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398306555
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
Before the server was only reused if the base path was the same it seems.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815152490
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b10a5699df7578ad8c6c6239bb887f50b6438e34 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1823590565
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
}, {
"hash" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096",
"triggerID" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"triggerType" : "PUSH"
}, {
"hash" : "cae921ac9d016d28b87139b5c0fd24debadf1592",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21109",
"triggerID" : "cae921ac9d016d28b87139b5c0fd24debadf1592",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* cae921ac9d016d28b87139b5c0fd24debadf1592 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21109)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1822243625
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bbf765005b4e1e92730e0dff736bcb561d928b7b Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815713303
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1877ef99bb4c939181b5341e19c44cbba742d7cd Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957)
* faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815236308
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b10a5699df7578ad8c6c6239bb887f50b6438e34 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956)
* 1877ef99bb4c939181b5341e19c44cbba742d7cd Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398108329
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
This is to properly close the write clients. Is there a better way to do this in the tests? It's my first time working in the flink tests
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399649058
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
Review Comment:
So, ideally we will have just 1 port and 1 timeline server right. unless someone explicitly configures diff port for timeline server when they are running multiple jobs against multiple tables.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
+ private static final Map<Integer, EmbeddedTimelineService> RUNNING_SERVICES = new HashMap<>();
+ private static final Registry METRICS_REGISTRY = Registry.getRegistry("TimelineService");
+ private static final String NUM_EMBEDDED_TIMELINE_SERVERS = "numEmbeddedTimelineServers";
private int serverPort;
private String hostAddr;
- private HoodieEngineContext context;
+ private final HoodieEngineContext context;
private final SerializableConfiguration hadoopConf;
private final HoodieWriteConfig writeConfig;
- private final String basePath;
+ private TimelineService.Config serviceConfig;
+ private final Set<String> basePaths; // the set of base paths using this EmbeddedTimelineService
private transient FileSystemViewManager viewManager;
private transient TimelineService server;
- public EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
+ private EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
setHostAddr(embeddedTimelineServiceHostAddr);
this.context = context;
this.writeConfig = writeConfig;
- this.basePath = writeConfig.getBasePath();
+ this.basePaths = new HashSet<>();
+ this.basePaths.add(writeConfig.getBasePath());
this.hadoopConf = context.getHadoopConf();
this.viewManager = createViewManager();
}
+ /**
+ * Returns an existing embedded timeline service if one is running for the given configuration and reuse is enabled, or starts a new one.
+ * @param context The {@link HoodieEngineContext} for the client
+ * @param embeddedTimelineServiceHostAddr The host address to use for the service (nullable)
+ * @param writeConfig The {@link HoodieWriteConfig} for the client
+ * @return A running {@link EmbeddedTimelineService}
+ * @throws IOException if an error occurs while starting the service
+ */
+ public static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) throws IOException {
+ return getOrStartEmbeddedTimelineService(context, embeddedTimelineServiceHostAddr, writeConfig, TimelineService::new);
+ }
+
+ static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig,
+ TimelineServiceCreator timelineServiceCreator) throws IOException {
+ // if reuse is enabled, check if any existing instances are compatible
+ if (writeConfig.isEmbeddedTimelineServerReuseEnabled()) {
+ synchronized (SERVICE_LOCK) {
+ for (EmbeddedTimelineService service : RUNNING_SERVICES.values()) {
+ if (service.canReuseFor(writeConfig, embeddedTimelineServiceHostAddr)) {
+ service.addBasePath(writeConfig.getBasePath());
+ LOG.info("Reusing existing embedded timeline server with configuration: " + service.serviceConfig);
+ return service;
+ }
+ }
+ // if no compatible instance is found, create a new one
+ EmbeddedTimelineService service = createAndStartService(context, embeddedTimelineServiceHostAddr, writeConfig, timelineServiceCreator);
+ RUNNING_SERVICES.put(service.serverPort, service);
Review Comment:
from the impl of canReuseFor, I feel, a triplet of host, port and markerType uniquely identified a timeline server.
For eg, lets say someone switching marker type for a table from DIRECT to TIMELINE SERVER based.
I see, we might have two timeline servers spinning up. But the source of truth i.e. the RUNNING_SERVICES map even though will have two entries, but the key is the same. or might override previous entry.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
+ private static final Map<Integer, EmbeddedTimelineService> RUNNING_SERVICES = new HashMap<>();
+ private static final Registry METRICS_REGISTRY = Registry.getRegistry("TimelineService");
+ private static final String NUM_EMBEDDED_TIMELINE_SERVERS = "numEmbeddedTimelineServers";
private int serverPort;
private String hostAddr;
- private HoodieEngineContext context;
+ private final HoodieEngineContext context;
private final SerializableConfiguration hadoopConf;
private final HoodieWriteConfig writeConfig;
- private final String basePath;
+ private TimelineService.Config serviceConfig;
+ private final Set<String> basePaths; // the set of base paths using this EmbeddedTimelineService
private transient FileSystemViewManager viewManager;
private transient TimelineService server;
- public EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
+ private EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
setHostAddr(embeddedTimelineServiceHostAddr);
this.context = context;
this.writeConfig = writeConfig;
- this.basePath = writeConfig.getBasePath();
+ this.basePaths = new HashSet<>();
+ this.basePaths.add(writeConfig.getBasePath());
this.hadoopConf = context.getHadoopConf();
this.viewManager = createViewManager();
}
+ /**
+ * Returns an existing embedded timeline service if one is running for the given configuration and reuse is enabled, or starts a new one.
+ * @param context The {@link HoodieEngineContext} for the client
+ * @param embeddedTimelineServiceHostAddr The host address to use for the service (nullable)
+ * @param writeConfig The {@link HoodieWriteConfig} for the client
+ * @return A running {@link EmbeddedTimelineService}
+ * @throws IOException if an error occurs while starting the service
+ */
+ public static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) throws IOException {
+ return getOrStartEmbeddedTimelineService(context, embeddedTimelineServiceHostAddr, writeConfig, TimelineService::new);
+ }
+
+ static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig,
+ TimelineServiceCreator timelineServiceCreator) throws IOException {
+ // if reuse is enabled, check if any existing instances are compatible
+ if (writeConfig.isEmbeddedTimelineServerReuseEnabled()) {
+ synchronized (SERVICE_LOCK) {
+ for (EmbeddedTimelineService service : RUNNING_SERVICES.values()) {
+ if (service.canReuseFor(writeConfig, embeddedTimelineServiceHostAddr)) {
+ service.addBasePath(writeConfig.getBasePath());
+ LOG.info("Reusing existing embedded timeline server with configuration: " + service.serviceConfig);
+ return service;
+ }
+ }
+ // if no compatible instance is found, create a new one
+ EmbeddedTimelineService service = createAndStartService(context, embeddedTimelineServiceHostAddr, writeConfig, timelineServiceCreator);
+ RUNNING_SERVICES.put(service.serverPort, service);
Review Comment:
oh, I also see metadata config props used. probably better to create an object with below entries
```
host, port, marker type, metadataEnableFlag.
```
So, for every unique combination of these we should have a diff timeline server instantiated.
In straight forward case, when we are running 100 tables w/ exact same values for (timeline service host, port, marker type and metadataEnableFlag), we should have just one timeline server running serving all 100 tables.
This aligns the canReuseFor() and the timeline servers we are running and tracking using
RUNNING_SERVICES
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +196,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
+ }
+
+ private boolean metadataConfigsAreEquivalent(Properties properties1, Properties properties2) {
+ Set<Object> metadataConfigs = new HashSet<>(properties1.keySet());
+ metadataConfigs.addAll(properties2.keySet());
+ return metadataConfigs.stream()
+ .filter(key -> ((String) key).startsWith(HoodieMetadataConfig.METADATA_PREFIX))
+ .allMatch(key -> {
+ String value1 = properties1.getProperty((String) key, "");
+ String value2 = properties2.getProperty((String) key, "");
+ return value1.equals(value2);
+ });
+
}
- public void stop() {
- if (null != server) {
+ /**
+ * Stops the embedded timeline service for the given base path. If a timeline service is managing multiple tables, it will only be shutdown once all tables have been stopped.
+ * @param basePath For the table to stop the service for
+ */
+ public void stopForBasePath(String basePath) {
Review Comment:
minor. the method not only stops for base path. also does some addtional clean up.
so, may be `cleanUpAndMaybeStopService(basePath)`
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -88,30 +148,20 @@ public void startServer() throws IOException {
.markerBatchIntervalMs(writeConfig.getMarkersTimelineServerBasedBatchIntervalMs())
.markerParallelism(writeConfig.getMarkersDeleteParallelism());
}
+ this.serviceConfig = timelineServiceConfBuilder.build();
- if (writeConfig.isEarlyConflictDetectionEnable()) {
Review Comment:
lets also include these to the key object in RUNNING_SERVICES.
isEarlyConflictDetectionEnabled.
isTimelineServerBasedInstantStateEnabled.
if not, we might keep re-using the same timeline server expecting certain behaviors based on these configs.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +196,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
+ }
+
+ private boolean metadataConfigsAreEquivalent(Properties properties1, Properties properties2) {
+ Set<Object> metadataConfigs = new HashSet<>(properties1.keySet());
+ metadataConfigs.addAll(properties2.keySet());
+ return metadataConfigs.stream()
Review Comment:
we just need if metadata is enabled or not. mainly used for FileSystemView type.
we don't need all properties of metadata table.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1821683706
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 697114b6ec4f578123363a89a6846e352bc3a53e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1823471080
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
}, {
"hash" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096",
"triggerID" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"triggerType" : "PUSH"
}, {
"hash" : "cae921ac9d016d28b87139b5c0fd24debadf1592",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "cae921ac9d016d28b87139b5c0fd24debadf1592",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 597f6d7bd7134d635ad5a675bd398ba03faafef8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096)
* cae921ac9d016d28b87139b5c0fd24debadf1592 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1820054386
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* c78d9570c87afce7a27725b37a3ebb77913199b7 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1822504410
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
}, {
"hash" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096",
"triggerID" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bbf765005b4e1e92730e0dff736bcb561d928b7b Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081)
* 597f6d7bd7134d635ad5a675bd398ba03faafef8 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21096)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan merged PR #10122:
URL: https://github.com/apache/hudi/pull/10122
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1397381007
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineServerHelper.java:
##########
@@ -23,66 +23,42 @@
import org.apache.hudi.common.util.Option;
import org.apache.hudi.config.HoodieWriteConfig;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.io.IOException;
/**
* Helper class to instantiate embedded timeline service.
*/
public class EmbeddedTimelineServerHelper {
- private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
- private static Option<EmbeddedTimelineService> TIMELINE_SERVER = Option.empty();
-
/**
* Instantiate Embedded Timeline Server.
* @param context Hoodie Engine Context
* @param config Hoodie Write Config
* @return TimelineServer if configured to run
* @throws IOException
*/
- public static synchronized Option<EmbeddedTimelineService> createEmbeddedTimelineService(
+ public static Option<EmbeddedTimelineService> createEmbeddedTimelineService(
HoodieEngineContext context, HoodieWriteConfig config) throws IOException {
- if (config.isEmbeddedTimelineServerReuseEnabled()) {
- if (!TIMELINE_SERVER.isPresent() || !TIMELINE_SERVER.get().canReuseFor(config.getBasePath())) {
- TIMELINE_SERVER = Option.of(startTimelineService(context, config));
- } else {
- updateWriteConfigWithTimelineServer(TIMELINE_SERVER.get(), config);
- }
- return TIMELINE_SERVER;
- }
if (config.isEmbeddedTimelineServerEnabled()) {
- return Option.of(startTimelineService(context, config));
+ Option<String> hostAddr = context.getProperty(EngineProperty.EMBEDDED_SERVER_HOST);
+ EmbeddedTimelineService timelineService = EmbeddedTimelineService.getOrStartEmbeddedTimelineService(context, hostAddr.orElse(null), config);
+ updateWriteConfigWithTimelineServer(timelineService, config);
Review Comment:
Yes, there is a [MultiTable](https://github.com/apache/hudi/blob/master/hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/HoodieMultiTableStreamer.java) version of the streamer.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815788365
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399754308
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
+ private static final Map<Integer, EmbeddedTimelineService> RUNNING_SERVICES = new HashMap<>();
+ private static final Registry METRICS_REGISTRY = Registry.getRegistry("TimelineService");
+ private static final String NUM_EMBEDDED_TIMELINE_SERVERS = "numEmbeddedTimelineServers";
private int serverPort;
private String hostAddr;
- private HoodieEngineContext context;
+ private final HoodieEngineContext context;
private final SerializableConfiguration hadoopConf;
private final HoodieWriteConfig writeConfig;
- private final String basePath;
+ private TimelineService.Config serviceConfig;
+ private final Set<String> basePaths; // the set of base paths using this EmbeddedTimelineService
private transient FileSystemViewManager viewManager;
private transient TimelineService server;
- public EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
+ private EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
setHostAddr(embeddedTimelineServiceHostAddr);
this.context = context;
this.writeConfig = writeConfig;
- this.basePath = writeConfig.getBasePath();
+ this.basePaths = new HashSet<>();
+ this.basePaths.add(writeConfig.getBasePath());
this.hadoopConf = context.getHadoopConf();
this.viewManager = createViewManager();
}
+ /**
+ * Returns an existing embedded timeline service if one is running for the given configuration and reuse is enabled, or starts a new one.
+ * @param context The {@link HoodieEngineContext} for the client
+ * @param embeddedTimelineServiceHostAddr The host address to use for the service (nullable)
+ * @param writeConfig The {@link HoodieWriteConfig} for the client
+ * @return A running {@link EmbeddedTimelineService}
+ * @throws IOException if an error occurs while starting the service
+ */
+ public static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) throws IOException {
+ return getOrStartEmbeddedTimelineService(context, embeddedTimelineServiceHostAddr, writeConfig, TimelineService::new);
+ }
+
+ static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig,
+ TimelineServiceCreator timelineServiceCreator) throws IOException {
+ // if reuse is enabled, check if any existing instances are compatible
+ if (writeConfig.isEmbeddedTimelineServerReuseEnabled()) {
+ synchronized (SERVICE_LOCK) {
+ for (EmbeddedTimelineService service : RUNNING_SERVICES.values()) {
+ if (service.canReuseFor(writeConfig, embeddedTimelineServiceHostAddr)) {
+ service.addBasePath(writeConfig.getBasePath());
+ LOG.info("Reusing existing embedded timeline server with configuration: " + service.serviceConfig);
+ return service;
+ }
+ }
+ // if no compatible instance is found, create a new one
+ EmbeddedTimelineService service = createAndStartService(context, embeddedTimelineServiceHostAddr, writeConfig, timelineServiceCreator);
+ RUNNING_SERVICES.put(service.serverPort, service);
Review Comment:
infact, if we introduce an object, easier to check for presence of timeline server. we can take the hash and store. and .containsKey() should suffice. as of now, we loop through all values.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399812787
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +196,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
+ }
+
+ private boolean metadataConfigsAreEquivalent(Properties properties1, Properties properties2) {
+ Set<Object> metadataConfigs = new HashSet<>(properties1.keySet());
+ metadataConfigs.addAll(properties2.keySet());
+ return metadataConfigs.stream()
+ .filter(key -> ((String) key).startsWith(HoodieMetadataConfig.METADATA_PREFIX))
+ .allMatch(key -> {
+ String value1 = properties1.getProperty((String) key, "");
+ String value2 = properties2.getProperty((String) key, "");
+ return value1.equals(value2);
+ });
+
}
- public void stop() {
- if (null != server) {
+ /**
+ * Stops the embedded timeline service for the given base path. If a timeline service is managing multiple tables, it will only be shutdown once all tables have been stopped.
+ * @param basePath For the table to stop the service for
+ */
+ public void stopForBasePath(String basePath) {
Review Comment:
I don't think the implementation needs to be reflected in the method name. We should focus on what the caller's intention is
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1819962855
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965)
* c78d9570c87afce7a27725b37a3ebb77913199b7 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1820519250
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* c78d9570c87afce7a27725b37a3ebb77913199b7 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029)
* 697114b6ec4f578123363a89a6846e352bc3a53e UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1401369779
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
responded below
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
+ private static final Map<Integer, EmbeddedTimelineService> RUNNING_SERVICES = new HashMap<>();
+ private static final Registry METRICS_REGISTRY = Registry.getRegistry("TimelineService");
+ private static final String NUM_EMBEDDED_TIMELINE_SERVERS = "numEmbeddedTimelineServers";
private int serverPort;
private String hostAddr;
- private HoodieEngineContext context;
+ private final HoodieEngineContext context;
private final SerializableConfiguration hadoopConf;
private final HoodieWriteConfig writeConfig;
- private final String basePath;
+ private TimelineService.Config serviceConfig;
+ private final Set<String> basePaths; // the set of base paths using this EmbeddedTimelineService
private transient FileSystemViewManager viewManager;
private transient TimelineService server;
- public EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
+ private EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
setHostAddr(embeddedTimelineServiceHostAddr);
this.context = context;
this.writeConfig = writeConfig;
- this.basePath = writeConfig.getBasePath();
+ this.basePaths = new HashSet<>();
+ this.basePaths.add(writeConfig.getBasePath());
this.hadoopConf = context.getHadoopConf();
this.viewManager = createViewManager();
}
+ /**
+ * Returns an existing embedded timeline service if one is running for the given configuration and reuse is enabled, or starts a new one.
+ * @param context The {@link HoodieEngineContext} for the client
+ * @param embeddedTimelineServiceHostAddr The host address to use for the service (nullable)
+ * @param writeConfig The {@link HoodieWriteConfig} for the client
+ * @return A running {@link EmbeddedTimelineService}
+ * @throws IOException if an error occurs while starting the service
+ */
+ public static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) throws IOException {
+ return getOrStartEmbeddedTimelineService(context, embeddedTimelineServiceHostAddr, writeConfig, TimelineService::new);
+ }
+
+ static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig,
+ TimelineServiceCreator timelineServiceCreator) throws IOException {
+ // if reuse is enabled, check if any existing instances are compatible
+ if (writeConfig.isEmbeddedTimelineServerReuseEnabled()) {
+ synchronized (SERVICE_LOCK) {
+ for (EmbeddedTimelineService service : RUNNING_SERVICES.values()) {
+ if (service.canReuseFor(writeConfig, embeddedTimelineServiceHostAddr)) {
+ service.addBasePath(writeConfig.getBasePath());
+ LOG.info("Reusing existing embedded timeline server with configuration: " + service.serviceConfig);
+ return service;
+ }
+ }
+ // if no compatible instance is found, create a new one
+ EmbeddedTimelineService service = createAndStartService(context, embeddedTimelineServiceHostAddr, writeConfig, timelineServiceCreator);
+ RUNNING_SERVICES.put(service.serverPort, service);
Review Comment:
responded below
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1823818490
<img width="1233" alt="image" src="https://github.com/apache/hudi/assets/513218/43a50fef-afef-4a80-b54a-75d5fe1260d3">
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "the-other-tim-brown (via GitHub)" <gi...@apache.org>.
the-other-tim-brown commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398209508
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
If you don't close the write clients, the embedded timeline servers will not shutdown. There were some test failures since these tests turn on reuse timeline server by default already but were expecting new servers to come up per run. I can roll back these changes if you want but hopefully we can find a way to close the closeables in tests.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1822491357
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
}, {
"hash" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "597f6d7bd7134d635ad5a675bd398ba03faafef8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* bbf765005b4e1e92730e0dff736bcb561d928b7b Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081)
* 597f6d7bd7134d635ad5a675bd398ba03faafef8 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399959143
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
I see why you did these changes, the `TestWriteCopyOnWrite#testReuseEmbeddedServer` can not pass because the embedded server started by other tests is resued, if we fix the reuse strategy to include the `FileSystemViewStorageConfig`, the test could pass.
But overall the fix makes sense, closing the client explicitly releases the resource.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1820533361
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* c78d9570c87afce7a27725b37a3ebb77913199b7 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029)
* 697114b6ec4f578123363a89a6846e352bc3a53e Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1822019706
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 697114b6ec4f578123363a89a6846e352bc3a53e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057)
* bbf765005b4e1e92730e0dff736bcb561d928b7b UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399754308
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -29,37 +31,95 @@
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.hudi.timeline.service.TimelineService;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Timeline Service that runs as part of write client.
*/
public class EmbeddedTimelineService {
+ // lock used when starting/stopping/modifying embedded services
+ private static final Object SERVICE_LOCK = new Object();
private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
+ private static final AtomicInteger NUM_SERVERS_RUNNING = new AtomicInteger(0);
+ // Map of port to existing timeline service running on that port
+ private static final Map<Integer, EmbeddedTimelineService> RUNNING_SERVICES = new HashMap<>();
+ private static final Registry METRICS_REGISTRY = Registry.getRegistry("TimelineService");
+ private static final String NUM_EMBEDDED_TIMELINE_SERVERS = "numEmbeddedTimelineServers";
private int serverPort;
private String hostAddr;
- private HoodieEngineContext context;
+ private final HoodieEngineContext context;
private final SerializableConfiguration hadoopConf;
private final HoodieWriteConfig writeConfig;
- private final String basePath;
+ private TimelineService.Config serviceConfig;
+ private final Set<String> basePaths; // the set of base paths using this EmbeddedTimelineService
private transient FileSystemViewManager viewManager;
private transient TimelineService server;
- public EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
+ private EmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) {
setHostAddr(embeddedTimelineServiceHostAddr);
this.context = context;
this.writeConfig = writeConfig;
- this.basePath = writeConfig.getBasePath();
+ this.basePaths = new HashSet<>();
+ this.basePaths.add(writeConfig.getBasePath());
this.hadoopConf = context.getHadoopConf();
this.viewManager = createViewManager();
}
+ /**
+ * Returns an existing embedded timeline service if one is running for the given configuration and reuse is enabled, or starts a new one.
+ * @param context The {@link HoodieEngineContext} for the client
+ * @param embeddedTimelineServiceHostAddr The host address to use for the service (nullable)
+ * @param writeConfig The {@link HoodieWriteConfig} for the client
+ * @return A running {@link EmbeddedTimelineService}
+ * @throws IOException if an error occurs while starting the service
+ */
+ public static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig) throws IOException {
+ return getOrStartEmbeddedTimelineService(context, embeddedTimelineServiceHostAddr, writeConfig, TimelineService::new);
+ }
+
+ static EmbeddedTimelineService getOrStartEmbeddedTimelineService(HoodieEngineContext context, String embeddedTimelineServiceHostAddr, HoodieWriteConfig writeConfig,
+ TimelineServiceCreator timelineServiceCreator) throws IOException {
+ // if reuse is enabled, check if any existing instances are compatible
+ if (writeConfig.isEmbeddedTimelineServerReuseEnabled()) {
+ synchronized (SERVICE_LOCK) {
+ for (EmbeddedTimelineService service : RUNNING_SERVICES.values()) {
+ if (service.canReuseFor(writeConfig, embeddedTimelineServiceHostAddr)) {
+ service.addBasePath(writeConfig.getBasePath());
+ LOG.info("Reusing existing embedded timeline server with configuration: " + service.serviceConfig);
+ return service;
+ }
+ }
+ // if no compatible instance is found, create a new one
+ EmbeddedTimelineService service = createAndStartService(context, embeddedTimelineServiceHostAddr, writeConfig, timelineServiceCreator);
+ RUNNING_SERVICES.put(service.serverPort, service);
Review Comment:
infact, if we introduce an object, easier to check for presence of timeline server. we can store the object as and .containsKey() should suffice. as of now, we loop through all values.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1822025391
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
}, {
"hash" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21029",
"triggerID" : "c78d9570c87afce7a27725b37a3ebb77913199b7",
"triggerType" : "PUSH"
}, {
"hash" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057",
"triggerID" : "697114b6ec4f578123363a89a6846e352bc3a53e",
"triggerType" : "PUSH"
}, {
"hash" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081",
"triggerID" : "bbf765005b4e1e92730e0dff736bcb561d928b7b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 697114b6ec4f578123363a89a6846e352bc3a53e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21057)
* bbf765005b4e1e92730e0dff736bcb561d928b7b Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=21081)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398207752
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
I'm trying to understand why we must close the fs view forceablly. Are there any errors thrown here if we not do this?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1399958120
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +214,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
Review Comment:
Should we actually also compare the `FileSystemViewStorageConfig` ? If there are any options falgged by specific client, we should not reuse it.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1398065544
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestWriteMergeOnReadWithCompact.java:
##########
@@ -159,6 +159,8 @@ public void testNonBlockingConcurrencyControlWithPartialUpdatePayload() throws E
// because the data files belongs 3rd commit is not included in the last compaction.
Map<String, String> readOptimizedResult = Collections.singletonMap("par1", "[id1,par1,id1,Danny,23,2,par1]");
TestData.checkWrittenData(tempFile, readOptimizedResult, 1);
+ pipeline1.end();
+ pipeline2.end();
Review Comment:
Is this an optimization or correctness fix?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1400959986
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -146,19 +214,65 @@ public FileSystemViewManager getViewManager() {
return viewManager;
}
- public boolean canReuseFor(String basePath) {
- return this.server != null
- && this.viewManager != null
- && this.basePath.equals(basePath);
+ /**
+ * Adds a new base path to the set that are managed by this instance.
+ * @param basePath the new base path to add
+ */
+ private void addBasePath(String basePath) {
+ basePaths.add(basePath);
+ }
+
+ private boolean canReuseFor(HoodieWriteConfig newWriteConfig, String newHostAddr) {
+ if (server == null || viewManager == null) {
+ return false; // service is not running
+ }
+ if (basePaths.contains(newWriteConfig.getBasePath())) {
+ return true; // already running for this base path
+ }
+ if (newHostAddr != null && !newHostAddr.equals(this.hostAddr)) {
+ return false; // different host address
+ }
+ if (writeConfig.getMarkersType() != newWriteConfig.getMarkersType()) {
+ return false; // different marker type
+ }
+ return metadataConfigsAreEquivalent(writeConfig.getMetadataConfig().getProps(), newWriteConfig.getMetadataConfig().getProps());
Review Comment:
I thought about this and also request handlers that tim pointed out. Here is my take on this.
By default we are going to disable reuse. So, users should enable only on a need basis. I will add more documentation to call out that we expect some homoegenous set up to use this.
For eg, in multi-table deltastreamer, if user has 10 tables, all 10 of them is expected to have similar config values for other secondary configs like num threads w/ timeline server, marker batch num threads, marker parallelism etc. If the 10 tables are going to have slightly diff values for these secondary configs, its better for user to not use this feature only.
As of now, the key properties that will determine whether one can reuse an existing timeline server or not is
- hostAddr
- metadata enable flag
- early conflict detection enabled
- marker type
- isTimelineServerBasedInstantStateEnabled
if these matches, hudi will reuse an existing timeline server. So the first one to instantiate the timeline server, the resp writeConfig will be applied(for sec configs like marker parallelism, threads, etc).
We can revisit this for key properties going forward. but may not be scalable/maintainable if we account for every minor config difference.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815718739
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
}, {
"hash" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965",
"triggerID" : "faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1877ef99bb4c939181b5341e19c44cbba742d7cd Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957)
* faf61fb4c40584fd9dbdd4aafc85e699c3d9d8ba Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20965)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10122:
URL: https://github.com/apache/hudi/pull/10122#issuecomment-1815290105
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20956",
"triggerID" : "b10a5699df7578ad8c6c6239bb887f50b6438e34",
"triggerType" : "PUSH"
}, {
"hash" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957",
"triggerID" : "1877ef99bb4c939181b5341e19c44cbba742d7cd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1877ef99bb4c939181b5341e19c44cbba742d7cd Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=20957)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7112] Reuse existing timeline server and performance improvements [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10122:
URL: https://github.com/apache/hudi/pull/10122#discussion_r1396674754
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineServerHelper.java:
##########
@@ -23,66 +23,42 @@
import org.apache.hudi.common.util.Option;
import org.apache.hudi.config.HoodieWriteConfig;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.io.IOException;
/**
* Helper class to instantiate embedded timeline service.
*/
public class EmbeddedTimelineServerHelper {
- private static final Logger LOG = LoggerFactory.getLogger(EmbeddedTimelineService.class);
-
- private static Option<EmbeddedTimelineService> TIMELINE_SERVER = Option.empty();
-
/**
* Instantiate Embedded Timeline Server.
* @param context Hoodie Engine Context
* @param config Hoodie Write Config
* @return TimelineServer if configured to run
* @throws IOException
*/
- public static synchronized Option<EmbeddedTimelineService> createEmbeddedTimelineService(
+ public static Option<EmbeddedTimelineService> createEmbeddedTimelineService(
HoodieEngineContext context, HoodieWriteConfig config) throws IOException {
- if (config.isEmbeddedTimelineServerReuseEnabled()) {
- if (!TIMELINE_SERVER.isPresent() || !TIMELINE_SERVER.get().canReuseFor(config.getBasePath())) {
- TIMELINE_SERVER = Option.of(startTimelineService(context, config));
- } else {
- updateWriteConfigWithTimelineServer(TIMELINE_SERVER.get(), config);
- }
- return TIMELINE_SERVER;
- }
if (config.isEmbeddedTimelineServerEnabled()) {
- return Option.of(startTimelineService(context, config));
+ Option<String> hostAddr = context.getProperty(EngineProperty.EMBEDDED_SERVER_HOST);
+ EmbeddedTimelineService timelineService = EmbeddedTimelineService.getOrStartEmbeddedTimelineService(context, hostAddr.orElse(null), config);
+ updateWriteConfigWithTimelineServer(timelineService, config);
Review Comment:
So a single Spark Driver has chance to be shared by multiple writers?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org