SteNicholas commented on code in PR #203:
URL: https://github.com/apache/paimon-cpp/pull/203#discussion_r3781203765
##########
src/paimon/common/factories/io_hook.cpp:
##########
@@ -49,9 +49,11 @@ class IOHook::Impl {
}
inline void Reset(int64_t pos, IOHook::Mode mode) {
+ // Store mode_ first: the seq_cst stores below then publish it, so a
Try()
+ // that observes the reset pos_ also observes the new mode_.
+ mode_.store(mode, std::memory_order_relaxed);
Review Comment:
The individual accesses are now data-race-free, but Reset() still does not
publish a coherent configuration. For example, starting from (SILENT, -1, 0),
Reset(INT64_MAX, RETURN_ERROR) can store the new mode, after which a concurrent
Try() increments the old counter, reads the old -1 position, and returns
IOError. Both stable configurations would return OK (the completed reset should
not fire until INT64_MAX), so this is a torn state rather than either valid
outcome. Since the new test explicitly supports concurrent Reset()/Try(),
please publish an immutable combined state including a fresh counter, or
synchronize the complete operations; a regression test can alternate Clear()
with Reset(INT64_MAX, RETURN_ERROR) and assert that Try() never fails.
--
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]
