FDB-723: remotefdb single-connection flush-blob ordering deadlock

[mcakircali]

Description

Tries to fix an deadlock in remote FDB when the client and server negotiate a single (shared) connection. Two CI tests are randomly timing out — fdb_test_remote_single_conn_stress (300s) and FDB-610 (600s).

On a single connection (shared a socket), client sends catalog (field-location) Blobs asynchronously via ClientConnection::dataWriteQueue_ (written later by the data-writer thread), but sends the blocking Flush msg directly.

The direct Flush may take over the queued Blobs. The server then reads Flush first, blocks in CatalogueHandler::flush() waiting, and cannot read the Blobs, which is a deadlock.

Fix

Add barrier registry + failDataBarriers() for data writes so the catalog flush cannot arrive before Blobs.

ClientConnection::flushDataWrites() adds a barrier item (std::promise<void>) to the data writes queue and blocks until the writer thread reaches it and guarantees queued Blobs arrive before blocking flush.

Related fixes

Add missing dataWriteQueue_ lock
Remove throw in thread; double throw bug

Contributor Declaration

By opening this pull request, I affirm the following:

• All authors agree to the Contributor License Agreement.
• The code follows the project's coding standards.
• I have performed self-review and added comments where needed.
• I have added or updated tests to verify that my changes are effective and functional.
• I have run all existing tests and confirmed they pass.

🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/fdb/pull-requests/PR-346

[codecov-commenter]

Codecov Report

❌ Patch coverage is 63.26531% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.46%. Comparing base (06d24c9) to head (e2d97cd).
⚠️ Report is 7 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/fdb5/remote/client/ClientConnection.cc	62.50%	18 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #346      +/-   ##
===========================================
- Coverage    77.60%   77.46%   -0.14%     
===========================================
  Files          411      411              
  Lines        27669    27711      +42     
  Branches      2771     2775       +4     
===========================================
- Hits         21473    21467       -6     
- Misses        6196     6244      +48     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🟡 Changes recommended

The new barrier implementation introduces concurrency hazards (data race / potential indefinite wait) that can lead to undefined behavior or hangs under teardown/exception scenarios.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses a deadlock in RemoteFDB single-connection mode by ensuring asynchronous “Blob” (data) writes are fully processed by the data-writer thread before issuing the blocking Flush control message on the shared socket.

Changes:

• Add a ClientConnection::flushDataWrites() barrier to ensure queued data writes are processed before a blocking flush.
• Invoke this barrier from RemoteCatalogue::flush() before sending Message::Flush.
• Extend the data-writer queue element to support barrier items via a std::promise<void>.

File summaries

File	Description
src/fdb5/remote/client/RemoteCatalogue.cc	Calls the new barrier before sending blocking Flush on a shared socket.
src/fdb5/remote/client/ClientConnection.h	Exposes flushDataWrites() as a public API for ordering data writes vs control messages.
src/fdb5/remote/client/ClientConnection.cc	Implements barrier queue items and the flushDataWrites() wait mechanism in the data-writer thread.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The changes alter multi-threaded connection behavior and need careful human validation under failure/stress scenarios to ensure no new hangs or regressions are introduced.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[danovaro]

thanks!