u70b3 commented on code in PR #203:
URL: https://github.com/apache/paimon-cpp/pull/203#discussion_r3819243989
##########
src/paimon/common/factories/io_hook.cpp:
##########
@@ -29,18 +31,19 @@ namespace paimon {
class IOHook::Impl {
public:
Status Try(const std::string& path) {
- if (io_count_.fetch_add(1) < pos_.load()) {
+ std::shared_lock<std::shared_mutex> lock(mutex_);
Review Comment:
Good point — done in 993b2d5 by gating the synchronized path behind an
atomic `armed` flag:
- `Try()` now starts with a lock-free fast path: a single acquire load,
returning OK immediately in the disabled state (the production default). The
`shared_mutex` is only taken once armed.
- `Reset()` publishes the complete configuration under the mutex and then
release-stores `armed`; armed readers acquire the mutex before touching
`mode_`/`pos_`, so an observed armed state always implies a complete
configuration.
- `Clear()` disarms first (still under the mutex) so IO threads drop off the
lock as soon as possible.
- One semantic change worth noting: the disabled fast path no longer
increments the counter, so `IOCount()` now counts only while armed. All
`IOCount()` consumers are tests, and the header documents the contract.
- Coverage: new `IOHookTest.TestDisabledFastPath` pins the disabled behavior
(Try always OK, no counting, re-arm/disarm restores it). The existing
`TestConcurrentResetAndTry` hammers `Reset(INT64_MAX, RETURN_ERROR)`/`Clear()`
continuously, so workers keep switching between the fast and armed paths under
TSan.
Validation on Kunpeng-920 (aarch64), head 993b2d5: factories suite 10/10
(plus 20× `--gtest_shuffle` and a `--gtest_repeat=5 --gtest_shuffle` run);
`paimon-common-test` 1450/1450; `paimon-common-sst-file-format-test` 32/32;
TSan clean over 20 shuffled stress runs of the race tests; pre-commit and `git
diff --check` clean.
--
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]
