Joy-2000 opened a new pull request, #19794:
URL: https://github.com/apache/hudi/pull/19794
### Describe the issue this Pull Request addresses
<!-- Either describe the issue inline here with motivation behind the
changes
(or) link to an issue by including `Closes #<issue-number>` for
context.
If this PR includes changes to the storage format, public APIs,
or has breaking changes, use `!` (e.g., feat!: ...) -->
This PR fixes two related defects in the metadata-table auto-lock path that
can cause silent lock failures for table-service writers.
**1. Default lock-path inconsistency (correctness / data-safety)**
`FileSystemBasedLockProvider.getLockConfig(tablePath)` derived the default
lock path from the `.hoodie/.aux` folder, while the provider's own constructor
fell back to the `.hoodie` meta folder when no path was configured. The two
defaults could resolve to **different directories** for the same table, so two
writers relying on different code paths would take locks on different files and
never actually exclude each other — defeating the lock across engines/tasks.
**2. CLI lock config never took effect (correctness)**
`RunClusteringProcedure` / `RunCompactionProcedure` appended the
auto-derived lock options to `confs` **after** the write client had already
been built from `confs`, so the lock configuration was silently dropped and
these table services could update the metadata table without the intended DFS
lock.
### Summary and Changelog
<!-- Short, plain-English summary of what users gain or what changed in
behavior.
Followed by a detailed log of all the changes. Highlight if any code
was copied. -->
Table-service writers launched through the SQL procedures (compaction,
clustering, and the other write procedures) now reliably acquire a
filesystem-based lock on a single, consistent path under the table's metadata
folder when the table has a metadata table and no lock provider is otherwise
configured.
Changes:
- `FileSystemBasedLockProvider`
- Introduce/reuse a single `defaultLockPath()` helper and change the
default lock path from the `.hoodie/.aux` folder to the table metadata path
`.hoodie` (`METAFOLDER_NAME`).
- The constructor's fallback now reuses `defaultLockPath()` so the
constructor fallback and `getLockConfig()` default can never diverge again.
- `HoodieCLIUtils.createHoodieWriteClient`
- Move the metadata-table auto-lock injection here, applied **after** the
final parameters are merged and **before** the client is built, so it actually
takes effect.
- Guard on the fully-merged parameters
(`!finalParameters.contains(LOCK_PROVIDER_CLASS_NAME)`) so any
explicitly-configured lock provider — at conf, table-config, or session level —
is respected.
- Refactor `getLockOptions(tablePath, scheme, params)` accordingly.
- This extends lock coverage from just clustering/compaction to **all**
write procedures that go through `createHoodieWriteClient`.
- `RunClusteringProcedure` / `RunCompactionProcedure`: remove the now-dead
post-build lock-injection blocks and unused imports.
- Tests: extend `TestFileSystemBasedLockProvider` (verify the explicit
`getLockConfig` path and the constructor `BASE_PATH` fallback resolve to the
same `.hoodie/lock` file) and add cases to `TestHoodieCLIUtils` for
`getLockOptions` (supported/null scheme → FS lock config under `.hoodie`,
unsupported scheme → empty, custom `hoodie.fs.atomic_creation.support` → FS
lock config).
### Impact
<!-- Describe any public API or user-facing feature change or any
performance impact. -->
- Behavior change: table-service writes via the SQL procedures now
auto-configure a filesystem lock on `.hoodie` (previously the config was
dropped, or could point at `.aux`). Users who already set a lock provider at
any layer are unaffected — their provider is preserved.
- No public API signature changes; `getLockOptions` is an internal utility
whose signature changed.
- No performance impact.
### Risk Level
<!-- Accepted values: none, low, medium or high. Other than `none`, explain
the risk.
If medium or high, explain what verification was done to mitigate the
risks. -->
low
- The auto-injection only triggers when the table has a metadata table
**and** no lock provider is configured at any layer, so existing explicit
configurations are untouched.
- `HoodieWriteConfig.AUTO_ADJUST_LOCK_CONFIGS` defaults to `false`, so the
injected FS lock provider is not downgraded to in-process.
- Verified by `TestFileSystemBasedLockProvider` (17/17 pass) and
`TestHoodieCLIUtils` (13/13 pass), including the new path-consistency and
`getLockOptions` cases.
### Documentation Update
<!-- Describe any necessary documentation update if there is any new
feature, config, or user-facing change. If not, put "none".
none — no new configs and no config default values are changed (the change
fixes the *derived default lock path* used internally, not a user-facing config
default).
- 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 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)
- [ ] Enough context is provided in the sections above
- [ ] 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]
