fix(hive-sync): drop the unreachable HMS lock timeout-recovery path

[wombatu-kun]

Describe the issue this Pull Request addresses

Closes #19345, point 1. Point 2 of that issue is addressed by #19370.

Summary and Changelog

An acquire that times out now reports the timeout instead of a misleading NoSuchLockException.

• Removed the catch (InterruptedException | TimeoutException e) block from HiveMetastoreBasedLockProvider.acquireLockInternal. Both exceptions are already declared by the method, so they propagate and tryLock wraps the actual TimeoutException.
• Collapsed the hoisted lockRequest local and its lockRequestFinal copy into one final local, since they only existed to feed checkLock after the try block.
• Added TestHiveMetastoreBasedLockProviderAcquireTimeout on top of the shared HiveMetastoreBasedLockProviderTestBase: with a mocked IMetaStoreClient whose lock() never answers, the acquire must fail with HoodieLockException caused by TimeoutException and must not call checkLock. Without this change that test fails with a bare NullPointerException.

Impact

HoodieLockException on an acquire timeout is now caused by TimeoutException rather than NoSuchLockException. No API or configuration change.

Risk Level

low

The removed code could not execute against a real metastore, so no working behaviour is taken away.

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

[wombatu-kun]

@voonhous could you please review this too.
To close hive-sync lock provider issue finally.

[hudi-agent]

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

Thanks for the contribution! This PR removes an unreachable HMS lock timeout-recovery branch so that an acquire timeout now reports a TimeoutException-caused HoodieLockException instead of a misleading error, and adds a focused test. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

[voonhous]

Reviewed the removal itself and it holds up. LockRequestBuilder.build() never sets txnid (only setTransactionId(long) does, and the provider never calls it), so getTxnid() returns 0 and TxnHandler.checkLock throws NoSuchLockException for lock id 0 -- lock ids come from NEXT_LOCK_ID, seeded at 1.

Worth adding to the PR body: there is a second, independent reason the branch was dead. Both callers of acquireLockInternal are gated by ValidationUtils.checkArgument(this.lock == null), and the only assignment to lock inside the try is the get(time, unit) that threw, so this.lock == null was always true at the catch site -- the state == ACQUIRED adopt-and-return branch could never be taken even if checkLock had been given the right id.

Two notes on the new test below.

[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! This PR removes the unreachable HMS lock timeout-recovery branch in HiveMetastoreBasedLockProvider.acquireLockInternal so an acquire timeout now surfaces a TimeoutException cause instead of a misleading NoSuchLockException. Tracing both the old and new paths, the removed branch could not recover against a real metastore (it looked up lockRequest.getTxnid(), which is 0 for a request never populated from a server response), and the server-granted-lock leak behavior is unchanged, so this is a net improvement. No new issues flagged from this automated pass beyond what other reviewers already noted on the test. 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.

cc @yihua

[wombatu-kun]

On the second reason: with this.lock == null invariant at the catch site the outer if (this.lock == null || this.lock.getState() != ACQUIRED) is a tautology, so what it kills is that second disjunct and the implicit fall-through that swallowed the exception when a lock was already held, while the inner adopt branch reads only what checkLock returns and a correct lock id would have made it reachable.

Leaving the body as is on that basis, since the txnid-0 argument stays the only reason the recovery was dead.

[hudi-bot]

CI report:

• fc893fd Azure: SUCCESS
• 1f19f77 Azure: PENDING

Bot commands

@hudi-bot supports the following commands:

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