GH-49433: [C++] Buffer ARROW_LOG output to prevent thread interleaving#49663
GH-49433: [C++] Buffer ARROW_LOG output to prevent thread interleaving#49663pitrou merged 3 commits intoapache:mainfrom
Conversation
|
|
github-actions
Bot
added
awaiting committer review
Shockp
force-pushed
the
fix-thread-safe-logging
branch
from
3566c7e to
ea6bbf5
Compare
|
Added the static std::mutex lock to the destuctor to protect the final flush, and verified it locally with a new 10-thread concurrent test in logging_test.cc. |
…leaving When using the ARROW_LOG macros from multiple threads, messages were getting mingled together in stderr because the operator<< was writing directly to the global stream piece-by-piece. This commit introduces an internal std::ostringstream buffer to CerrLog so that messages are accumulated locally and flushed atomically upon destruction.
pitrou
force-pushed
the
fix-thread-safe-logging
branch
from
ea6bbf5 to
fdb92d6
Compare
|
Ok, it looks like |
|
Thank you so much for the review and the final tweaks, @pitrou! I really appreciate the guidance on my first PR. I will keep contributing more. |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 5633a18. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them. |
2 participants
Rationale for this change
When using the C++
ARROW_LOGmacros from multiple threads, messages often get mingled together instderrbecause theoperator<<writes directly to the global stream piece-by-piece, which is not thread-safe.What changes are included in this PR?
This PR introduces an internal
std::ostringstreambuffer to the fallbackCerrLogclass.Instead of writing to
std::cerrimmediately on every<<operation, the messages are accumulated locally within theCerrLoginstance. The completed string is then flushed atomically tostd::cerrupon the object's destruction.Are these changes tested?
Yes. I built the C++ project locally and ran the test suite (
ctest) to ensure no existing logging behavior or IPC functionality was broken.Are there any user-facing changes?
No API changes. Users will simply notice that multi-threaded
ARROW_LOGconsole output is now cleanly separated per line instead of interleaved.Closes #49433