linliu-code opened a new pull request, #20207:
URL: https://github.com/apache/hudi/pull/20207
### Describe the issue this Pull Request addresses
This PR only adds tests. They show that two threads sharing one
`TransactionManager` can both be inside a state change at once when the lock
provider is `StorageBasedLockProvider`, and that the second thread then
releases the first thread's lock. I'm opening it as a draft so we can agree the
problem is real before discussing a fix. The tests fail on master on purpose.
How two threads end up sharing one `TransactionManager`:
- HoodieStreamer gives its ingest write client to the in-process async
compactor and clusterer (`HoodieStreamer#onInitializingWriteClient` creates
`SparkAsyncCompactService(hoodieSparkContext, writeClient)`, and
`HoodieSparkCompactor` calls `writeClient.compact` and
`writeClient.commitCompaction` on that object). Spark streaming does the same
in `HoodieStreamingSink`.
- The compactor thread commits through the write client's table service
client: `completeCompaction`, then `txnManager.beginStateChange`.
- The ingest thread also uses that table service client to schedule
compaction and clustering, to roll back failed writes and to schedule cleaning
(`scheduleTableServiceInternal`, `rollbackFailedWrites`, `scheduleCleaning`).
So both threads take the lock through the same `TransactionManager`,
`LockManager` and lock provider instance.
Why only `StorageBasedLockProvider`:
- `StorageBasedLockProvider.tryLock()` returns true whenever *this provider
instance* already holds the lock (`if (actuallyHoldsLock()) return true; //
Supports reentrant locks`), whichever thread asks. The second thread therefore
enters without waiting.
- Whichever thread ends its state change first expires the lock file, while
the other thread is still committing.
- `InProcessLockProvider` and the ZooKeeper provider own the lock per
thread, so the second thread waits.
`TestTransactionManager#testMultiWriterTransactions` already expects that wait,
but it only runs with `InProcessLockProvider`.
### Summary and Changelog
Tests only, no production code change:
-
`TestTransactionManagerSharedAcrossThreads#secondThreadWaitsAndCannotReleaseHeldLock`,
parameterized over the lock provider:
- One thread holds the lock inside a compaction state change.
- A second thread on the same `TransactionManager` begins a state change.
- The test asserts that the second thread did not enter, and that an
independent writer cannot acquire the lock while the first thread is still
inside.
- `InProcessLockProvider` is the control and passes.
`StorageBasedLockProvider` fails both assertions on master.
- `InMemoryStorageBasedLockProvider` (test helper):
`StorageBasedLockProvider` with its locking logic unchanged, backed by
in-memory storage with the same conditional-write semantics as S3, GCS and
Azure.
-
`TestHoodieDeltaStreamer#testAsyncCompactorSharesIngestWriteClientAndTransactionManager`:
in continuous MOR mode with async compaction, the async compactor holds the
very same write client and `TransactionManager` as the ingest thread.
How the tests were run: TestTransactionManagerSharedAcrossThreads ran
locally against master. The InProcessLockProvider case passes; the
StorageBasedLockProvider case fails both assertions ("a second thread entered
the state change while another thread held the lock" and "another writer
acquired the lock while the first thread was still inside its state change").
The TestHoodieDeltaStreamer case needs Docker for the class's test containers
and was not run locally; CI runs it here for the first time.
### Impact
None; tests only. On master, the `StorageBasedLockProvider` case of the new
test fails until there is a fix.
### Risk Level
none
### Documentation Update
none
### Contributor's checklist
- [x] 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]
