gavinchou commented on PR #64574: URL: https://github.com/apache/doris/pull/64574#issuecomment-4721974532
Thanks for the fix. Since `BthreadSharedMutex` becomes a new correctness-critical synchronization primitive, I think it is worth adding a small dedicated UT instead of only relying on the existing tablet tests. Suggested coverage: - Basic `SharedMutex` contract: multiple concurrent `std::shared_lock`s can coexist; `std::unique_lock` is exclusive; `try_lock` / `try_lock_shared` fail while the opposite mode is held. - Writer-pending behavior of the two-gate algorithm: once a writer has set the write-entered bit and is waiting for existing readers to drain, new readers should block until the writer has acquired and released the lock. - Cross-worker / cross-thread unlock regression case: acquire the lock in a bthread, suspend/yield enough to allow worker migration, and unlock from the resumed context. If possible, also include a deterministic variant using two pthreads/bthreads to demonstrate that ownership is not tied to the original OS thread. - Mixed reader/writer stress test with invariant counters, e.g. `active_writers <= 1` and `active_writers == 0` whenever `active_readers > 0`, running many bthreads with both shared and exclusive sections. - RAII compile/runtime coverage with `std::shared_lock<BthreadSharedMutex>` and `std::unique_lock<BthreadSharedMutex>`, because the PR relies on being a drop-in replacement at call sites. The original bug is subtle and platform-dependent, so a focused `be/test/util/bthread_shared_mutex_test.cpp` would make this much easier to keep safe during future changes. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
