fhan688 opened a new pull request, #20395:
URL: https://github.com/apache/hudi/pull/20395
### Describe the issue this Pull Request addresses
`FileSystemBasedLockProvider` does not track whether a provider instance
successfully acquired the lock.
Both `unlock()` and `close()` may delete the shared lock file without
checking local acquisition state. This can break mutual exclusion in the
following sequence:
1. Provider A successfully acquires the filesystem lock.
2. Provider B attempts to acquire the same lock and fails.
3. Provider B is closed and deletes the shared lock file.
4. Provider C successfully acquires the lock while Provider A is still
executing its critical section.
A repeated `unlock()` from a provider that has already released its lock can
similarly delete a successor's lock.
This PR addresses the deterministic unowned-release scenario.
Generation-safe expiry reclamation and fencing are intentionally outside the
scope of this change.
### Summary and Changelog
This PR makes filesystem lock release dependent on successful acquisition by
the same provider instance.
Changes include:
- Track successful lock acquisition using transient provider-local state.
- Mark the lock as acquired only after exclusive file creation, payload
writing, and stream closing complete successfully.
- Return acquisition success directly instead of using lock-file existence
as proof of ownership.
- Make `unlock()` a no-op when the provider did not successfully acquire the
lock.
- Make `close()` release the lock only when the provider owns it while still
closing the underlying storage.
- Clear the local acquisition state only after the lock is successfully
released or is already absent.
- Preserve the existing non-reentrant behavior for repeated `tryLock()`
calls.
- Add regression coverage for:
- Closing a failed contender while another provider holds the lock.
- Unlocking a failed contender while another provider holds the lock.
- Repeated unlock after a successor acquires the lock.
- Releasing an owned lock through `close()`.
### Impact
There are no public API, configuration, or storage-format changes.
The behavior of `FileSystemBasedLockProvider` changes as follows:
- A provider that never acquired the lock can no longer delete the shared
lock file through `unlock()` or `close()`.
- A provider that already released its lock can no longer delete a
successor's lock through a repeated `unlock()`.
- A provider that currently owns the lock continues to release it through
either `unlock()` or `close()`.
This PR does not address stale-owner release after expiry-based lock
reclamation. Solving that safely across processes requires generation-aware
conditional operations and fencing and should be handled separately.
### Risk Level
**Medium**
This change modifies lock lifecycle behavior in
`FileSystemBasedLockProvider`, but the scope is limited to preventing unowned
lock deletion.
Risk is mitigated by:
- Keeping the existing exclusive-create acquisition mechanism unchanged.
- Preserving owner-driven `unlock()` and `close()` behavior.
- Keeping acquisition state transient and internal to the provider.
- Adding regression tests for failed contenders, repeated release, and owner
close.
- Running the complete `TestFileSystemBasedLockProvider` test class
successfully.
Validation result:
```text
Tests run: 21, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
Validation command:
mvn -pl hudi-client/hudi-client-common -am \
-Punit-tests -Pjava17 \
-DskipITs \
-Dcheckstyle.skip=false \
-Drat.skip=true \
-Djacoco.skip=true \
-Dtest=TestFileSystemBasedLockProvider \
-Dsurefire.failIfNoSpecifiedTests=false \
-DfailIfNoTests=false \
test
```
### Documentation Update
None. This is an internal correctness fix with no new configuration or
public API.
### 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
