voonhous commented on PR #19486:
URL: https://github.com/apache/hudi/pull/19486#issuecomment-5372988122
Notes for reviewers and the merging committer, moved out of the description
to keep it readable.
**Mutation ledger.** Every production change is pinned by a test that was
run against the reverted change to confirm it fails: the three monitor blocks
(each reverted alone times out the interned-literal test), `create(path, true)`
fails the atomic-create test, the pre-fix reload fails the vanished-lock-file
test, a widened `catch (IOException)` fails the IO-failure test, a
throw-and-clear catch fails its keep-the-owner assertion, non-transient
`lockInfo` fails the round trip with `NotSerializableException`, the removed
`""` initializer fails the failing-create test, and both `checkIfExpired`
mutants (treat-as-expired, rethrow) fail the stat-failure test.
**Serialization compatibility.** 0.12.x is the only release line that ever
serialized this class (HUDI-5377 landed in 0.13.0). Its streams carry a
computed `serialVersionUID` (`1687524493539720009`) that no 0.13.0+ build could
read anyway (0.13.0 computes `-7877409577963519069`), and no Hudi code persists
a provider outside intra-job closure capture. Pinning `1L` therefore cannot
break a readable stream.
**Squash message.** This branch's four commit messages tell a
self-correcting story (the third commit's message contains two claims its
successor corrects), and the default concatenated squash body would carry all
of them into master verbatim. Please replace the squash message body with:
```
FileSystemBasedLockProvider synchronized tryLock/unlock/close on the "lock"
String
literal. Compile-time constants are interned, so any class in the JVM
synchronizing
on the same literal contends on the very same monitor; the in-repo test
helper
FileSystemBasedLockProviderTestClass did exactly that, waiting on the
monitor the
production provider takes. The guard is a private static Object now, same
scope
within a classloader, no aliasing. The test helper got the same fix.
Also fixed while in the file, each pinned by a test that fails without it:
- reloadCurrentOwnerLockInfo evaluated storage.open in its try-with-resources
header, so a vanished lock file threw out of the reload instead of
clearing the
field, and LockManager reported the previous owner as current. The reload
now
treats FileNotFoundException (at open, or at first read on lazily fetching
stores) as "no owner" and lets any other IOException escape as
HoodieIOException.
- The class declared Serializable but every instance since 0.13.0 (HUDI-5377)
failed to serialize: lockInfo held a non-Serializable LockInfo built
eagerly in
the constructor, breaking Spark closure capture (the HUDI-7782 bug class).
lockInfo and sdf are transient and lazily rebuilt under the monitor;
serialVersionUID is pinned at 1L.
- currentOwnerLockInfo is volatile (getCurrentOwnerLockInfo is public
LockProvider
API and may be read from another thread) and defaults to "" so LockManager
never
logs a null owner.
- acquireLock is package-private @VisibleForTesting: storage.create(path,
false)
is the provider's cross-process mutual exclusion on stores with atomic
create,
and no public-API test can reach its already-exists arm.
- checkIfExpired documents that a stat failure degrades to "not expired",
the same
fail-safe choice as StorageBasedLockProvider's UNKNOWN_ERROR arm, and its
catch
logs ACQUIRING instead of the contradictory ALREADY_RELEASED.
- HoodieStorage.open documents the FileNotFoundException contract the reload
relies on; TestHoodieStorageBase already asserts it.
New tests: interned-monitor non-blocking for tryLock/unlock/close,
atomic-create
loser with the winner's payload intact, tryLock returning false when the
create
fails, reload clearing on a vanished lock file, reload keeping the last known
owner on a real IO error, a stat failure during the expiry check neither
stealing
a live lock nor skipping the owner reload, and a Java serialization round
trip of
a lock-holding provider.
Part of #16943
```
--
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]
