[MINOR] Release every client resource on close even when an earlier one fails

[PDGGK]

Describe the issue this Pull Request addresses

close() on the write client releases several independent resources as a bare sequence, so the first failure strands everything after it.

// BaseHoodieClient:153
stopEmbeddedServerView(true);
this.context.setJobStatus("", "");
this.heartbeatClient.close();
this.txnManager.close();

// BaseHoodieWriteClient:1663
super.close();
this.index.close();
this.tableServiceClient.close();

The one that matters most is last in the base chain: txnManager owns the write lock. If stopping the timeline server or closing the heartbeat client throws, the lock provider is never released, and the next writer waits for it to time out rather than getting a clean handover. The heartbeat client is in the same position — it holds a scheduled executor and leaves heartbeat files behind.

BaseHoodieWriteClient inherits the problem and adds to it: a failure in super.close() also skips the index and the table service client.

Summary and Changelog

Each step is now closed independently. The first failure is what the caller sees, with any later ones attached via addSuppressed rather than dropped, so nothing that used to surface stops surfacing.

Tests: TestBaseHoodieWriteClient#testCloseReleasesLaterResourcesWhenAnEarlierCloseFails injects an index whose close() throws and asserts the table service client and the transaction manager are still closed, and that the original failure is what propagates.

Reverting the change turns it red with Wanted but not invoked: tableServiceClient.close().

A note on how the test is shaped: an earlier version made the last resource throw, which passed with and without the fix — everything before it had already been released, so it proved nothing. The failure has to be injected ahead of the resources whose release is being asserted.

mvn test -pl hudi-client/hudi-client-common -Dtest=TestBaseHoodieWriteClient#testCloseReleasesLaterResourcesWhenAnEarlierCloseFails passes; checkstyle:check clean. The full TestBaseHoodieWriteClient class exhausts the heap on my machine both with and without this change, so I could not run it end to end locally.

Impact

No public API change. close() still throws the same failure it threw before; it simply releases the remaining resources first.

Risk Level

low

Documentation Update

none

Contributor's checklist

• Read through contributor's guide
• Enough context is provided in the sections above
• Adequate tests were added if applicable

[hudi-bot]

CI report:

• 5a31478 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

• @hudi-bot run azure re-run the last Azure build

[hudi-agent]

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! The PR makes close() in BaseHoodieClient and BaseHoodieWriteClient release each resource independently, so a failure in one step no longer strands the rest — with the first failure propagating and later ones attached via addSuppressed. The exception-accumulation and rethrow logic traces correctly (first failure preserved as primary, suppressed exceptions survive the HoodieException wrap, no NPE or self-suppression risk since all resources are non-null and distinct), and continuing to release txnManager (the write lock) on earlier failures is a clear improvement. One non-blocking note in the inline comments about test coverage for the base-chain path that the description highlights as most important. Please take a look at any inline comments, and this should be ready for a Hudi committer or PMC member to take it from here. One naming inconsistency worth a quick look.

[hudi-agent]

🤖 The new test covers the subclass path (index fails -> tableServiceClient still closes), but the base chain the description calls out as most important — a failure in stopEmbeddedServerView/heartbeatClient still releasing txnManager's write lock — isn't directly exercised (super.close() fully succeeds in that test). Might be worth a small test for that path too, e.g. inject a failure into stopEmbeddedServerView and assert txnManager.close() still runs.

_{⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.}

[hudi-agent]

🤖 nit: addSuppressed does the same thing as appendFailure in the base class, just with a different name and without the null-guard (so the call sites need an inline ternary). Could you either promote appendFailure to protected and reuse it here, or at least align the name so the two close() paths are visually consistent?

_{⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.}