yihua opened a new pull request, #8658: URL: https://github.com/apache/hudi/pull/8658
### Change Logs This PR fixes a bug introduced by #6847. #6847 extends the `InProcessLockProvider` to support multiple tables in the same process, by having an in-memory static final map storing the mapping of the table base path to the read-write reentrant lock, so that the writer uses the corresponding lock based on the base path. When closing the lock provider, `close()` removes the lock entry. Since `close()` is called when closing the write client, the lock is removed and subsequent concurrent writers will get a different lock instance on the same table, causing the locking mechanism on the same table to be useless. Take the following example where three writers write to the same table concurrently and need to acquire the in-process lock: ``` Writer 1: lock |----------------| unlock and close Writer 2: try lock | ... lock |------| unlock and close Writer 3: try lock | ... lock |------| unlock and close ``` after Writer 1 releases the lock and closes the lock provider, the lock instance is removed from the map, and Writer 3 will get a different lock instance compared to Writer 2. The fix gets rid of the lock removal operation in the `close()` call since it has to be kept for concurrent writers. This bug is uncovered while investigating the flaky test `testArchivalWithMultiWriters` (HUDI-6176) where concurrent cannot properly do archival due to the ineffective locking mechanism, although archival is guarded by locks. A new test `TestInProcessLockProvider#testLockIdentity` based on the above scenario is added to guard the behavior. Before this fix, the test failed because of varying lock instances (see logs below); after the fix, the test succeeds. ``` 1594 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 1 tries to acquire the lock. 1600 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 0, Read locks = 0], Thread ForkJoinPool-1-worker-9, In-process lock state ACQUIRING 1600 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 1, Read locks = 0], Thread ForkJoinPool-1-worker-9, In-process lock state ACQUIRED 1600 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 1 acquires the lock. 1601 [Thread-1] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 2 tries to acquire the lock. 1601 [Thread-1] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 1, Read locks = 0], Thread Thread-1, In-process lock state ACQUIRING 2604 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 1, Read locks = 0], Thread ForkJoinPool-1-worker-9, In-process lock state RELEASING 2605 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 0, Read locks = 0], Thread ForkJoinPool-1-worker-9, In-process lock state RELEASED 2605 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 1 releases the lock. 2605 [Thread-1] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 1, Read locks = 0], Thread Thread-1, In-process lock state ACQUIRED 2605 [Thread-1] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 2 acquires the lock. 2605 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 1, Read locks = 0], Thread ForkJoinPool-1-worker-9, In-process lock state ALREADY_RELEASED 2605 [ForkJoinPool-1-worker-9] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 1 closes the lock provider. 2615 [Thread-2] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 3 tries to acquire the lock. 2616 [Thread-2] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@380655c9[Write locks = 0, Read locks = 0], Thread Thread-2, In-process lock state ACQUIRING 2616 [Thread-2] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@380655c9[Write locks = 1, Read locks = 0], Thread Thread-2, In-process lock state ACQUIRED 2616 [Thread-2] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 3 acquires the lock. 3608 [Thread-1] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 1, Read locks = 0], Thread Thread-1, In-process lock state RELEASING 3608 [Thread-1] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 0, Read locks = 0], Thread Thread-1, In-process lock state RELEASED 3608 [Thread-1] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 2 releases the lock. 3609 [Thread-1] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 0, Read locks = 0], Thread Thread-1, In-process lock state ALREADY_RELEASED 3609 [Thread-1] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 2 closes the lock provider. 3619 [Thread-2] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@380655c9[Write locks = 1, Read locks = 0], Thread Thread-2, In-process lock state RELEASING 3619 [Thread-2] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@380655c9[Write locks = 0, Read locks = 0], Thread Thread-2, In-process lock state RELEASED 3619 [Thread-2] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 3 releases the lock. 3619 [Thread-2] INFO org.apache.hudi.client.transaction.lock.InProcessLockProvider [] - Base Path table1, Lock Instance java.util.concurrent.locks.ReentrantReadWriteLock@380655c9[Write locks = 0, Read locks = 0], Thread Thread-2, In-process lock state ALREADY_RELEASED 3619 [Thread-2] INFO org.apache.hudi.client.transaction.TestInProcessLockProvider [] - Writer 3 closes the lock provider. org.opentest4j.AssertionFailedError: Expected :java.util.concurrent.locks.ReentrantReadWriteLock@6978e791[Write locks = 0, Read locks = 0] Actual :java.util.concurrent.locks.ReentrantReadWriteLock@380655c9[Write locks = 0, Read locks = 0] ``` ### Impact Fixes a bug in `InProcessLockProvider` that may make in-process lock useless on the same table. ### Risk level low ### Documentation Update We need to update the release notes to mention this regression. ### Contributor's checklist - [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute) - [ ] Change Logs and Impact were stated clearly - [ ] Adequate tests were added if applicable - [ ] CI passed -- 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]
