You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "fhan688 (via GitHub)" <gi...@apache.org> on 2026/09/29 23:19:11 UTC
[PR] fix(core): reference-count shared embedded timeline services per table [hudi]
fhan688 opened a new pull request, #20147:
URL: https://github.com/apache/hudi/pull/20147
### Describe the issue this Pull Request addresses
`EmbeddedTimelineService` tracks active table paths using a `Set<String>`. When multiple clients in the same JVM independently acquire the same reusable service for the same table, their acquisitions collapse into a single entry.
Closing one client removes that entry and can shut down the service while another client still depends on it. If other tables keep the service alive, the unconditional `unregisterBasePath()` call still prematurely clears the affected table's file-system view.
The shutdown path also checks and closes the server outside the lifecycle lock, allowing competing releases to attempt duplicate closure. Removing a registry entry by identifier alone can evict a different service instance with the same identifier.
### Summary and Changelog
Keep a shared embedded timeline service available until all owning clients have released their references.
- Replace the base-path set with per-base-path reference counts.
- Clear a table's file-system view only when its last reference is released.
- Serialize reference updates and view cleanup with service acquisition, preventing an earlier release from clearing a newly acquired table view.
- Close the service only after all table references have been released.
- Detach the server under `SERVICE_LOCK` and close it outside the lock, assigning closure responsibility to a single caller.
- Remove registry entries only when they still point to the instance being released.
- Apply the same detach-and-close lifecycle to `shutdownAllTimelineServers()`, preventing later client releases from closing the server or decrementing its metric again.
- Continue releasing resources when view cleanup or server closure throws.
- Add regression tests for shared-table references, multiple tables, reacquisition, concurrent release, replacement services, forced shutdown, and cleanup failures.
### Impact
- Prevents premature service shutdown and file-system view cleanup for clients sharing the same table.
- Preserves the existing distinction between owned and borrowed services: each successful factory acquisition requires one release; borrowed services do not acquire an additional ownership reference.
- Does not change public method signatures, configuration defaults, dependencies, or storage formats.
- Keeps full server shutdown outside the global lifecycle lock. Table-view cleanup now runs under that lock to serialize it with reacquisition, which may increase acquisition/release latency when view cleanup is slow.
- Does not change the set of services managed by `shutdownAllTimelineServers()`.
### Risk Level
medium
This change affects shared-service lifecycle and synchronization, including concurrent acquisition, release, and forced shutdown.
Validation was performed with JDK 11 and the Spark 3.5 build profile:
```bash
mvn -Pspark3.5 \
-pl hudi-client/hudi-client-common -am \
-DskipITs \
-Dtest=TestEmbeddedTimelineService,TestTimelineService \
-DfailIfNoTests=false \
-Dsurefire.failIfNoSpecifiedTests=false \
test-compile surefire:test
```
Results:
- `TestEmbeddedTimelineService`: 16 tests passed, including 9 new test invocations.
- `TestTimelineService`: 2 tests passed.
- No failures, errors, or skipped tests.
- Compilation, Checkstyle, and license checks passed.
- `git diff --check` passed.
Concurrency tests use latches and bounded waits to exercise lifecycle transitions. The full project test suite was not run.
### Documentation Update
none
This fixes existing lifecycle behavior without introducing new features or configuration options. Method documentation now clarifies acquisition and release responsibilities.
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-5902241216
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "33016cb95c3b650dd99038db385318f188e651f6",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17366",
"triggerID" : "33016cb95c3b650dd99038db385318f188e651f6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 33016cb95c3b650dd99038db385318f188e651f6 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17366)
<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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #20147:
URL: https://github.com/apache/hudi/pull/20147#discussion_r4139376523
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -226,43 +225,71 @@ public FileSystemViewStorageConfig getRemoteFileSystemViewConfig(HoodieWriteConf
}
/**
- * Adds a new base path to the set that are managed by this instance.
- * @param basePath the new base path to add
+ * Acquires a reference for the given base path. Must be called while holding SERVICE_LOCK.
+ * @param basePath the base path to acquire
*/
private void addBasePath(String basePath) {
- basePaths.add(basePath);
+ basePathReferenceCounts.merge(basePath, 1, Integer::sum);
}
/**
- * 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
+ * Releases one acquisition for the given base path. The table's view is cleared only after its last
+ * reference is released, and the service is closed only after all references have been released.
+ * Each owner must release at most once per acquisition; borrowed services must not be released.
Review Comment:
π€ With reference counts, every owner now has to release exactly once. `HoodieCatalogUtil.alterTable` never closes the reuse-enabled client it gets from `createWriteClientV2`, so it would now keep the shared service alive after the other owners close. `StreamSync.close()` can also run twice without clearing `embeddedTimelineService`. Could you close the catalog client with try-with-resources and reset that field, here or in a follow-up?
<sub><i>β οΈ AI-generated; verify before applying. React π/π to flag quality.</i></sub>
--
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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-5901151011
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "33016cb95c3b650dd99038db385318f188e651f6",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17366",
"triggerID" : "33016cb95c3b650dd99038db385318f188e651f6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 33016cb95c3b650dd99038db385318f188e651f6 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17366)
<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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 closed pull request #20147: fix(core): reference-count shared embedded timeline services per table
URL: https://github.com/apache/hudi/pull/20147
--
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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-6055044963
hi @danny0405 @cshuo , could you please help review this PR when you have time? thanks!
--
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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-6076523238
> "When multiple clients in the same JVM independently acquire the same reusable service for the same table, their acquisitions collapse into a single entry.
>
> Closing one client removes that entry and can shut down the service while another client still depends on it"
>
> Just disable the reuse the timeline service in this case and always try to use remote timeline service as much as possible.
Thanks for pointing this out. Just to confirm, do you suggest disabling embedded timeline service reuse for Flink-created clients, which currently enable it unconditionally in FlinkWriteClients, and relying on a remote timeline service when sharing is needed?
If so, I will close this PR and follow up with a smaller change to disable reuse in the affected Flink path.
--
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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 closed pull request #20147: fix(core): reference-count shared embedded timeline services per table
URL: https://github.com/apache/hudi/pull/20147
--
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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-5901100650
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20147?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `55.81395%` with `19 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.40%. Comparing base ([`23a62e4`](https://app.codecov.io/gh/apache/hudi/commit/23a62e4fe6bc688425c0b5fb2da5c951c7038971?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`33016cb`](https://app.codecov.io/gh/apache/hudi/commit/33016cb95c3b650dd99038db385318f188e651f6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/20147?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../hudi/client/embedded/EmbeddedTimelineService.java](https://app.codecov.io/gh/apache/hudi/pull/20147?src=pr&el=tree&filepath=hudi-client%2Fhudi-client-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fclient%2Fembedded%2FEmbeddedTimelineService.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1jbGllbnQtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2NsaWVudC9lbWJlZGRlZC9FbWJlZGRlZFRpbWVsaW5lU2VydmljZS5qYXZh) | 55.81% | [15 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/hudi/pull/20147?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #20147 +/- ##
============================================
- Coverage 80.41% 80.40% -0.02%
Complexity 34900 34900
============================================
Files 2546 2546
Lines 142929 142948 +19
Branches 17381 17384 +3
============================================
- Hits 114940 114939 -1
- Misses 20074 20090 +16
- Partials 7915 7919 +4
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.97% <ΓΈ> (ΓΈ)` | |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.42% <55.81%> (-0.06%)` | :arrow_down: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.75% <ΓΈ> (ΓΈ)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.85% <ΓΈ> (ΓΈ)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.19% <ΓΈ> (ΓΈ)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.05% <ΓΈ> (ΓΈ)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.99% <ΓΈ> (ΓΈ)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.02% <ΓΈ> (ΓΈ)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.50% <ΓΈ> (ΓΈ)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.14% <ΓΈ> (ΓΈ)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ΓΈ> (ΓΈ)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/20147/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ΓΈ> (ΓΈ)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20147/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/20147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.99% <55.81%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/20147?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [.../hudi/client/embedded/EmbeddedTimelineService.java](https://app.codecov.io/gh/apache/hudi/pull/20147?src=pr&el=tree&filepath=hudi-client%2Fhudi-client-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fclient%2Fembedded%2FEmbeddedTimelineService.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jbGllbnQvaHVkaS1jbGllbnQtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2NsaWVudC9lbWJlZGRlZC9FbWJlZGRlZFRpbWVsaW5lU2VydmljZS5qYXZh) | `82.66% <55.81%> (-12.76%)` | :arrow_down: |
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-5901070799
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "33016cb95c3b650dd99038db385318f188e651f6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "33016cb95c3b650dd99038db385318f188e651f6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 33016cb95c3b650dd99038db385318f188e651f6 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] fix(core): reference-count shared embedded timeline services per table [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #20147:
URL: https://github.com/apache/hudi/pull/20147#issuecomment-6072674102
"When multiple clients in the same JVM independently acquire the same reusable service for the same table, their acquisitions collapse into a single entry.
Closing one client removes that entry and can shut down the service while another client still depends on it"
Just disable the reuse the timeline service in this case and always try to use remote timeline service as much as possible.
--
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