fix(linstor): reconnect only the stalled peer in the satellite plunger by yankawai · Pull Request #4184 · cozystack/cozystack

europrinter (yankawai) · 2026-09-09T15:00:18Z

What

drbd_stall_act in packages/system/linstor/hack/plunger/plunger-satellite.sh disconnects a single peer with drbdadm disconnect res:peer and then calls drbdadm connect res, which walks every connection of the resource. Each peer that is still connected makes drbdsetup connect fail with Device has a net-config (use disconnect first) (exit code 10), the helper only logs WARN: action failed for res:peer, and the peer that was just disconnected is never reconnected.

Observed

cozystack 1.4.6 sandbox, LINSTOR 1.33.2 / DRBD 9.3.2 on Talos 1.13.6, resync of a 20 GiB volume while the Primary keeps writing:

STALL detected: res=pvc-9d99d1b9-1efe-4340-bbb3-13421c6bc7f2 sync_peer=sb-cs0203 percent_in_sync=90.4 -> disconnect/connect
pvc-9d99d1b9-1efe-4340-bbb3-13421c6bc7f2: Failure: (125) Device has a net-config (use disconnect first)
Command 'drbdsetup connect pvc-9d99d1b9-1efe-4340-bbb3-13421c6bc7f2 5' terminated with exit code 10
WARN: action failed for pvc-9d99d1b9-1efe-4340-bbb3-13421c6bc7f2:sb-cs0203

Fix

Reconnect the same res:peer pair that was disconnected. Verified on the sandbox by patching the live linstor-plunger ConfigMap: the peer reconnects and the resync completes.

Note

Not part of this change: the stall detector compares percent-in-sync, which drbdsetup status --json reports with one decimal, so a slow resync (for example one throttled down to c-min-rate by application I/O) can look stalled for STALL_ITERS iterations and trigger a reconnect every couple of minutes. Happy to follow up on that separately if you think it is worth it.

Summary by CodeRabbit

Bug Fixes
- Improved reconnection behavior by targeting the explicitly selected resource-peer endpoint instead of reconnecting only at the resource level.

coderabbitai · 2026-09-09T15:00:37Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 1e9d5200-fc1e-4337-a288-c4eec702b999

📥 Commits

Reviewing files that changed from the base of the PR and between 56e10b1 and 2beab48.

📒 Files selected for processing (1)

packages/system/linstor/hack/plunger/plunger-satellite.sh

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

The stalled-resync recovery action now reconnects the explicitly selected resource-peer endpoint.

Changes

Stalled resync recovery

Layer / File(s)	Summary
Reconnect the selected peer endpoint `packages/system/linstor/hack/plunger/plunger-satellite.sh`	`drbd_stall_act` now calls `drbdadm connect "${res}:${peer}"` instead of reconnecting only the resource.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 8c649

Stalled resync recovery now reconnects only the peer it disconnected, avoiding resource-wide reconnect conflicts. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: reconnecting only the stalled peer in the LINSTOR satellite plunger.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

scooby87

LGTM (independent cozy-review pass).

The fix is correct. disconnect targets a single peer connection (res:peer) while the old connect $res reconnected the whole resource; in the multi-peer stall scenario the whole-resource connect always hits an already-connected peer and fails with exit 10, so the stalled peer stays disconnected forever. Making connect symmetric (res:peer) fixes exactly that. set -e is safe (the line ends in || log) and the operation is data-safe (no backing-storage touch).

Non-blocking note: there is no bats regression test for the plunger scripts (the harness has historically been absent). An invariant test with a stubbed drbdadm would lock down "disconnect and connect target the same res:peer".

Aleksei Sviridkin (lexfrei)

LGTM.

drbdadm takes a connection as resource:connection-name, and with a bare resource it walks every connection of that resource (drbdadm-9.0(8)). So the old line really did reach peers it had not disconnected.

I checked the actual arguments, not just the shape of the line: cut the script at the main loop, source it, stub drbdadm, and drbd_stall_act pvc-abc sb-node-2 90.4 prints disconnect pvc-abc:sb-node-2 followed by connect pvc-abc:sb-node-2. Same pair both times, and not swapped. $peer comes from connections[].name in drbdsetup status --json, which is the peer node name, and that is what a connection name defaults to. Line 92 is also the only connect or disconnect call in the package, so nothing else carries the same asymmetry.

Two things, neither blocking. Your "never reconnected" only follows if drbdadm stops iterating after a failed connection, and I did not confirm that in the source, so the sandbox log is the whole evidence and I can't rerun it. It doesn't change the fix, which is right either way, but this body becomes the merge commit message. And nothing tests the script: the bats runner only picks up hack/*.bats at the repo root, so a test would live there instead of next to the package. The probe above is most of one. Not worth holding a one-line fix for.

The percent-in-sync granularity you mention at the end is a separate problem, leaving it out of scope here.

Aleksei Sviridkin (lexfrei)

LGTM, same verdict. One merge-readiness fact to add.

The two required contexts, pre-commit and E2E Tests, have never posted on this head. Every workflow run on it sits in action_required, which is what a fork PR does until a maintainer approves the runs, so the only checks that reported are size, label and DCO. My approval rests on reading the script and on the stubbed drbdadm probe, not on CI — there is no CI evidence here either way. Those runs need approving before the required checks can report.

drbd_stall_act disconnects one peer with "drbdadm disconnect res:peer" but then runs "drbdadm connect res", which walks every connection of the resource. The peers that are still connected make drbdsetup fail with "Device has a net-config (use disconnect first)" (exit code 10), the helper only logs "WARN: action failed" and the stalled peer stays disconnected. Reconnect the same res:peer pair that was just disconnected. Signed-off-by: Yan Bondarenko <[email protected]>

github-actions · 2026-09-18T09:38:45Z

Successfully created backport PR for release-1.6:

[Backport release-1.6] fix(linstor): reconnect only the stalled peer in the satellite plunger #4319

…in the satellite plunger (#4319) # Description Backport of #4184 to `release-1.6`.

europrinter (yankawai) requested review from IvanHunters, Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei), Timofei Larkin (lllamnyp), mattia-eleuteri, myasnikovdaniil and Kirill Ilin (sircthulhu) as code owners September 9, 2026 15:00

github-actions Bot added area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug size/XS This PR changes 0-9 lines, ignoring generated files labels Sep 9, 2026

scooby87 approved these changes Sep 17, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) approved these changes Sep 17, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) force-pushed the fix/linstor-plunger-reconnect-peer branch from 2beab48 to 8c649df Compare September 18, 2026 01:54

Aleksei Sviridkin (lexfrei) mentioned this pull request Sep 18, 2026

ci: a re-run or late approval of a fork PR run tests a tree that exists in no branch #4312

Open

Aleksei Sviridkin (lexfrei) added the kind/backport Categorizes issue or PR as requiring a backport to the current release line label Sep 18, 2026

Aleksei Sviridkin (lexfrei) merged commit 74c8724 into cozystack:main Sep 18, 2026
17 of 18 checks passed

github-actions Bot mentioned this pull request Sep 18, 2026

[Backport release-1.6] fix(linstor): reconnect only the stalled peer in the satellite plunger #4319

Merged

myasnikovdaniil added a commit that referenced this pull request Sep 23, 2026

[Backport release-1.6] fix(linstor): reconnect only the stalled peer …

8697ecc

…in the satellite plunger (#4319) # Description Backport of #4184 to `release-1.6`.

Navigation Menu

Repository navigation

fix(linstor): reconnect only the stalled peer in the satellite plunger - #4184

fix(linstor): reconnect only the stalled peer in the satellite plunger#4184
Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:mainfrom
yankawai:fix/linstor-plunger-reconnect-peer

europrinter (yankawai) commented Sep 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

scooby87 left a comment

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

europrinter (yankawai) commented Sep 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Observed

Fix

Note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

scooby87 left a comment

Choose a reason for hiding this comment

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Choose a reason for hiding this comment

Uh oh!

Aleksei Sviridkin (lexfrei) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

europrinter (yankawai) commented Sep 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading