harden remotefdb tests by mcakircali · Pull Request #333 · ecmwf/fdb

mcakircali · 2026-08-13T12:57:12Z

Description

It's about detecting startup and cleanup issues (missing/orphaned servers). In addition, wheel build ci has been consistently red as they may run in parallel.

Improve remote FDB test-server lifecycles and parallelism.

Changed ports to:
FDB-610: moved to 8002/8003
FDB-595: 11000/11001
FDB-491: 12000/12001
Start each test fdb-server in an isolated process group, wait for its configured port to be ready, and terminate the whole group during cleanup.
Replace fixed startup sleeps with bounded readiness checks.
Give independent remote regressions dedicated CTest resource locks.
Preserve serialization only where tests still share an endpoint or state.

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-333

Copilot

Pull request overview

This PR hardens the remote-FDB test infrastructure by improving fdb-server lifecycle management (startup readiness + teardown) and enabling more parallel execution by reducing unnecessary global serialization.

Changes:

Updated regression test ports (e.g., FDB-610 now uses 8002/8003) and aligned dependent configs/scripts.
Replaced fixed startup sleeps with bounded readiness checks and started servers in isolated process groups for more reliable cleanup.
Split a shared CTest RESOURCE_LOCK into more granular locks, and added dedicated multi-store “tools” configs to avoid state/dir collisions.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/regressions/FDB-610/fdb-remote.yaml	Updates remote endpoint port for the FDB-610 client config.
tests/regressions/FDB-610/FDB-610.sh.in	Starts servers via `setsid` + readiness checks; improved teardown logic (needs a failure-path cleanup fix).
tests/regressions/FDB-610/fdb_config_store.yaml	Updates store server port to match new test port allocation.
tests/regressions/FDB-610/fdb_config_catalogue.yaml	Updates catalogue server port and its store endpoint reference.
tests/regressions/FDB-610/CMakeLists.txt	Moves FDB-610 regression to its own CTest resource lock.
tests/regressions/FDB-595/FDB-595.sh.in	Adds process-group startup + readiness checks (needs a failure-path cleanup fix).
tests/regressions/FDB-595/CMakeLists.txt	Moves FDB-595 regression to its own CTest resource lock.
tests/regressions/FDB-491/CMakeLists.txt	Moves FDB-491 regression to its own CTest resource lock.
tests/regressions/FDB-419/CMakeLists.txt	Moves FDB-419 regression to its own CTest resource lock.
tests/fdb/remote/test_server.sh.in	Adds readiness tracking and process-group cleanup for remote API test servers.
tests/fdb/remote/single_conn_stress/test_server.sh.in	Mirrors readiness + process-group cleanup for single-connection stress servers.
tests/fdb/remote/multi_store/tools_store1.yaml.in	New isolated store1 config for the multi-store “tools” workflow.
tests/fdb/remote/multi_store/tools_store2.yaml.in	New isolated store2 config for the multi-store “tools” workflow.
tests/fdb/remote/multi_store/tools_client.yaml.in	New isolated client config for the multi-store “tools” workflow.
tests/fdb/remote/multi_store/multi_store.sh.in	Adds readiness checks and process-group cleanup for the API multi-store test.
tests/fdb/remote/multi_store/multi_store_tools.sh.in	Adds readiness checks and process-group cleanup; switches to isolated roots/pidfiles for tools workflow.
tests/fdb/remote/multi_store/CMakeLists.txt	Configures new “tools” YAMLs and assigns distinct resource locks for API vs tools tests.

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

codecov-commenter · 2026-08-13T15:15:05Z

Codecov Report

❌ Patch coverage is 86.20690% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.55%. Comparing base (e5a5a47) to head (777ed96).

Files with missing lines	Patch %	Lines
...mote/single_conn_stress/test_single_conn_stress.cc	88.46%	6 Missing ⚠️
src/fdb5/remote/server/CatalogueHandler.cc	66.66%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #333      +/-   ##
===========================================
+ Coverage    77.51%   77.55%   +0.03%     
===========================================
  Files          411      411              
  Lines        27553    27612      +59     
  Branches      2769     2778       +9     
===========================================
+ Hits         21358    21414      +56     
- Misses        6195     6198       +3

☔ 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.

caraghbiner

There is quite a lot changing here, I am not clear which problems are being solved (is killing of servers not working/flaky on the CI currently?).

Also does this work on mac? I ask because I tried to use process groups in the past in these tests but with little success: the combination of ctest + OS-dependent process management + FDB's fork() behaviour often ended up with orphaned processes in the end.

caraghbiner · 2026-08-13T15:46:06Z

    ENVIRONMENT    "${test_environment}"
    TEST_PROPERTIES
-    RESOURCE_LOCK fdb_remote_tests  # Prevent concurrent runs of remote tests
+    RESOURCE_LOCK fdb_remote_fdb610


What does this do? The point of the remote tests sharing a resource lock was to prevent them running in parallel, to avoid many simultaneous processes on the CI.

what was the reason to avoid many simultaneous procs on CI?

caraghbiner · 2026-08-13T16:00:57Z

Indeed, I see setsid in the tests which is not on mac.

mcakircali · 2026-08-14T06:53:15Z

There is quite a lot changing here, I am not clear which problems are being solved (is killing of servers not working/flaky on the CI currently?).

Also does this work on mac? I ask because I tried to use process groups in the past in these tests but with little success: the combination of ctest + OS-dependent process management + FDB's fork() behaviour often ended up with orphaned processes in the end.

I updated the description with a summary of problems.

it's a good point to check macos support. thanks for the review.

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.

 set -ux

+source "@FDB_REMOTE_TEST_HELPER@"


 set -ux

+source "@FDB_REMOTE_TEST_HELPER@"


 set -ux

+source "@FDB_REMOTE_TEST_HELPER@"


mcakircali added 5 commits August 13, 2026 14:35

test(remote): harden

10dccac

test(remote): harden 595

e4d9458

test(remote): harden 610

f14724d

test(remote): harden 419

b0bf3fb

test(remote): harden 491

c6b9442

mcakircali requested a lite review from Copilot August 13, 2026 12:57

Copilot started reviewing on behalf of mcakircali August 13, 2026 12:57 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread tests/regressions/FDB-610/FDB-610.sh.in Outdated

Comment thread tests/regressions/FDB-595/FDB-595.sh.in Outdated

mcakircali added 3 commits August 13, 2026 15:42

test(remote): address pr comments

4763bcd

test(remote): replace log readiness

793d6c7

fix(remote): cat handler wipes in progress mutex

e30c087

mcakircali requested a review from danovaro August 13, 2026 14:39

caraghbiner requested changes Aug 13, 2026

View reviewed changes

mcakircali added 7 commits August 14, 2026 12:34

test(remote): add archive + flush ordering

db82869

test(remote): remote test helper

3c6dad1

test(remote): add tsan surpressions

ed1d8fb

Merge branch 'develop' into feature/remote-test-harden

f0e7bba

Merge branch 'develop' into feature/remote-test-harden

25ce11f

Merge branch 'develop' into feature/remote-test-harden

b73c0e5

remove resource lock

17d3ae4

danovaro requested a balanced review from Copilot August 25, 2026 14:05

Copilot started reviewing on behalf of danovaro August 25, 2026 14:06 View session

Merge branch 'develop' into feature/remote-test-harden

fbc27ef

Copilot AI reviewed Aug 25, 2026

View reviewed changes

limit # of attempts in fdb_remote_reap_parent

777ed96

Navigation Menu

Uh oh!

harden remotefdb tests - #333

harden remotefdb tests#333
mcakircali wants to merge 17 commits into
developfrom
feature/remote-test-harden

mcakircali commented Aug 13, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 13, 2026 •

edited

Loading

Uh oh!

caraghbiner left a comment

Uh oh!

caraghbiner Aug 13, 2026

Uh oh!

mcakircali Aug 14, 2026

Uh oh!

caraghbiner commented Aug 13, 2026

Uh oh!

mcakircali commented Aug 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

mcakircali commented Aug 13, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Declaration

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

caraghbiner left a comment

Choose a reason for hiding this comment

Uh oh!

caraghbiner Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

mcakircali Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

caraghbiner commented Aug 13, 2026

Uh oh!

mcakircali commented Aug 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mcakircali commented Aug 13, 2026 •

edited by github-actions Bot

Loading

codecov-commenter commented Aug 13, 2026 •

edited

Loading