Skip to content

csi: add VolumeLocks to NodeGetVolumeStats for RBD and CephFS#6291

Merged
mergify[bot] merged 1 commit into
ceph:develfrom
Rakshith-R:node-get-stats-lock
May 21, 2026
Merged

csi: add VolumeLocks to NodeGetVolumeStats for RBD and CephFS#6291
mergify[bot] merged 1 commit into
ceph:develfrom
Rakshith-R:node-get-stats-lock

Conversation

@Rakshith-R

@Rakshith-R Rakshith-R commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Acquire a targetPath lock in NodeGetVolumeStats to prevent running
multiple NodeGetVolumeStats call on same target path at once. This may
happen when ceph cluster has problems. This also matches the semantics
of locking in other gRPC calls.

Assisted-by: Claude noreply@anthropic.com

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@Rakshith-R Rakshith-R force-pushed the node-get-stats-lock branch 2 times, most recently from 51838af to caf57d2 Compare May 21, 2026 04:35
@black-dragon74

black-dragon74 commented May 21, 2026

Copy link
Copy Markdown
Member

The only issue I could think of is this RPC is rather too frequent. We should be fine though as the locks are in-memory?

iPraveenParihar
iPraveenParihar reviewed May 21, 2026
View reviewed changes
Comment thread internal/cephfs/nodeserver.go
return nil, status.Error(codes.InvalidArgument, err.Error())
}

if acquired := ns.VolumeLocks.TryAcquire(targetPath); !acquired {

@iPraveenParihar iPraveenParihar May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NodeUnpublishVolume uses targetPath to take lock, if healthchecker hungs NodeUnpublishVolume is blocked. Is that what we want?

@Rakshith-R Rakshith-R May 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NodeUnpublishVolume uses targetPath to take lock, if healthchecker hungs NodeUnpublishVolume is blocked. Is that what we want?

It'll get killed in 10 mins, it's fine and
If health check hangs, even unmap will hang too.

@Rakshith-R

Rakshith-R commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@Mergifyio queue

@mergify

mergify Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge Queue Status

🛑 Queue command has been cancelled

@iPraveenParihar

iPraveenParihar commented May 21, 2026

Copy link
Copy Markdown
Contributor

Run e2e after #6286 is merged.

@Rakshith-R

Rakshith-R commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@Mergifyio rebase

@Rakshith-R @ceph-csi-bot
csi: add VolumeLocks to NodeGetVolumeStats for RBD and CephFS
84f6d64 
Acquire a targetPath lock in NodeGetVolumeStats to prevent running
multiple NodeGetVolumeStats call on same target path at once. This may
happen when ceph cluster has problems. This also matches the semantics
of locking in other gRPC calls.

Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Rakshith R <rar@redhat.com>
@ceph-csi-bot ceph-csi-bot force-pushed the node-get-stats-lock branch from caf57d2 to 84f6d64 Compare May 21, 2026 11:08
@mergify

mergify Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Deprecation notice: This pull request comes from a fork and was rebased using bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the rebase action will no longer be able to rebase fork pull requests with this configuration. Please switch to the update action/command to ensure compatibility going forward.

@mergify

mergify Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

rebase

✅ Branch has been successfully rebased

@Rakshith-R

Rakshith-R commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@Mergifyio queue

@mergify

mergify Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

queue

☑️ Command queue ignored because it is already running from a previous command.

@Rakshith-R Rakshith-R added the ok-to-test Label to trigger E2E tests label May 21, 2026
@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.34

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.34

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.35

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/mini-e2e/k8s-1.34

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.35

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/mini-e2e/k8s-1.35

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.33

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.33

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/mini-e2e/k8s-1.33

@ceph-csi-bot

ceph-csi-bot commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot added ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. and removed ok-to-test Label to trigger E2E tests labels May 21, 2026
@mergify mergify Bot removed the ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. label May 21, 2026
@mergify

mergify Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Deprecation notice: This pull request comes from a fork and was queued with update_method=rebase and update_bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the merge queue will no longer be able to rebase fork pull requests with this configuration. To avoid disruption, switch to update_method=merge in your queue rule.

@mergify mergify Bot added the queued label May 21, 2026
@mergify

mergify Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-05-21 14:16 UTC · Rule: default
  • ✅ Checks skipped · PR is already up-to-date
  • ✅ Merged — 2026-05-21 14:17 UTC · at 84f6d641e423aa128240e24e7903f9531d44a5a9 · rebase

This pull request spent 52 seconds in the queue, including 8 seconds running CI.

Required conditions to merge

@mergify mergify Bot merged commit 45d14a3 into ceph:devel May 21, 2026
44 checks passed
@mergify mergify Bot removed the queued label May 21, 2026
@mergify mergify Bot mentioned this pull request May 21, 2026
Open
This was referenced Jun 3, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iPraveenParihar iPraveenParihar iPraveenParihar left review comments

@black-dragon74 black-dragon74 black-dragon74 approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

Assignees

No one assigned

Labels

backport-to-release-v3.16

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Rakshith-R @black-dragon74 @iPraveenParihar @ceph-csi-bot @Madhu-1