mds,client: correct last_issue struct fields and consistently send last_issue in cap messages by batrick · Pull Request #60283 · ceph/ceph

batrick · 2024-10-12T23:52:06Z

First set of changes I'd like to do for

https://tracker.ceph.com/issues/66704

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

A const getter already exists. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

Originally, the last_sent sequence from the MDS was sent by the client during bulk cap release but it was shortly after changed to the last_issue which is the sequence number that the cap was originally "issued" by the MDS rank (which may be updated after import of caps). Fixes: 6208f57 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

The peer seq is used as the issue_seq. Use that name for consistency. Fixes: 4fdeb00 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

Right now only the clients tell the MDS what they believe the issue_seq to be. The clients are expected to figure out issue_seq updates at Fixes: https://tracker.ceph.com/issues/68515 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick · 2024-10-14T20:04:07Z

jenkins test api

batrick · 2024-10-14T20:04:12Z

jenkins test make check arm64

batrick · 2024-10-19T00:57:25Z

This PR is under test in https://tracker.ceph.com/issues/68629.

* refs/pull/60283/head: mds: add issue_seq to all cap messages include/ceph_fs: correct ceph_mds_cap_peer field name include/ceph_fs: correct ceph_mds_cap_item field name messages/MClientCaps: use correct ceph_seq_t for cap sequence types messages/MClientCaps: dump issue_seq for debugging mds: remove dead code

batrick · 2024-10-22T00:50:50Z

https://tracker.ceph.com/issues/68629#note-1

vshankar · 2024-10-29T11:20:28Z

(On this today)

batrick · 2024-10-31T00:41:29Z

This PR is under test in https://tracker.ceph.com/issues/68786.

batrick · 2024-11-06T14:21:36Z

This PR is under test in https://tracker.ceph.com/issues/68859.

batrick · 2024-11-13T03:08:28Z

https://tracker.ceph.com/issues/68859#note-2

idryomov · 2024-11-18T11:19:19Z

src/client/Client.cc

-	    tcap.seq = m->peer.seq - 1;
-	    tcap.issue_seq = tcap.seq;
+	    tcap.seq = m->peer.issue_seq - 1;
+	    tcap.issue_seq = tcap.issue_seq;


@batrick I was taking a look at "ceph: correct ceph_mds_cap_peer field name" patch for the kernel client and there is something fishy here:

previously both clients effectively assigned m->peer.seq - 1 to tcap.seq and tcap.issue_seq

m->peer.seq got renamed to m->peer.issue_seq

kernel client continues to assign that value to both fields

tcap->seq = t_issue_seq - 1; tcap->issue_seq = t_issue_seq - 1;

userspace client assigns that value only to tcap.seq, for tcap.issue_seq there appears to be a bogus self-assignment

@batrick I was taking a look at "ceph: correct ceph_mds_cap_peer field name" patch for the kernel client and there is something fishy here:

* previously both clients effectively assigned `m->peer.seq - 1` to `tcap.seq` and `tcap.issue_seq`

Correct.

* `m->peer.seq` got renamed to `m->peer.issue_seq`

Correct.

* kernel client continues to assign that value to both fields

tcap->seq = t_issue_seq - 1; tcap->issue_seq = t_issue_seq - 1;

* userspace client assigns that value only to `tcap.seq`, for `tcap.issue_seq` there appears to be a bogus self-assignment

Oh, very nice catch. My find/replace clearly messed that up.

Credit to Ilya Dryomov for spotting this. [1] ceph#60283 (comment) Fixes: 1da6ef2 Fixes: https://tracker.ceph.com/issues/68973 Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>

Credit to Ilya Dryomov for spotting this. [1] ceph#60283 (comment) Fixes: 1da6ef2 Fixes: https://tracker.ceph.com/issues/68973 Signed-off-by: Patrick Donnelly <pdonnell@ibm.com> (cherry picked from commit a07c5ef)

Credit to Ilya Dryomov for spotting this. [1] ceph/ceph#60283 (comment) Fixes: 1da6ef2 Fixes: https://tracker.ceph.com/issues/68973 Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>

Credit to Ilya Dryomov for spotting this. [1] ceph#60283 (comment) Fixes: 1da6ef2 Fixes: https://tracker.ceph.com/issues/68973 Signed-off-by: Patrick Donnelly <pdonnell@ibm.com> (cherry picked from commit a07c5ef)

batrick added 5 commits October 10, 2024 08:53

mds: remove dead code

a72b31e

A const getter already exists. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

messages/MClientCaps: dump issue_seq for debugging

7766d3c

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

messages/MClientCaps: use correct ceph_seq_t for cap sequence types

6d8a734

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

include/ceph_fs: correct ceph_mds_cap_peer field name

1da6ef2

The peer seq is used as the issue_seq. Use that name for consistency. Fixes: 4fdeb00 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

github-actions bot added the cephfs Ceph File System label Oct 12, 2024

mds: add issue_seq to all cap messages

cb4ff28

Right now only the clients tell the MDS what they believe the issue_seq to be. The clients are expected to figure out issue_seq updates at Fixes: https://tracker.ceph.com/issues/68515 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick force-pushed the i66704 branch from eab7050 to cb4ff28 Compare October 13, 2024 00:41

batrick added the wip-pdonnell-testing2 label Oct 15, 2024

batrick requested a review from a team October 22, 2024 00:51

batrick added the needs-review label Oct 22, 2024

vshankar self-assigned this Oct 25, 2024

batrick added wip-pdonnell-testing and removed wip-pdonnell-testing2 labels Oct 25, 2024

vshankar approved these changes Nov 4, 2024

View reviewed changes

batrick unassigned vshankar Nov 5, 2024

batrick added needs-qa and removed needs-review labels Nov 5, 2024

batrick merged commit 3829eb6 into ceph:main Nov 13, 2024

batrick deleted the i66704 branch November 13, 2024 03:13

idryomov reviewed Nov 18, 2024

View reviewed changes

idryomov mentioned this pull request Nov 18, 2024

client: resolve bogus self-assignment #60761

Merged

14 tasks

This was referenced Jan 9, 2025

reef: mds,client: correct last_issue struct fields and consistently send last_issue in cap messages #61294

Closed

squid: mds,client: correct last_issue struct fields and consistently send last_issue in cap messages #61300

Merged

joscollin mentioned this pull request Oct 3, 2025

reef: mds,client: correct last_issue struct fields and consistently send last_issue in cap messages #65768

Open

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mds,client: correct last_issue struct fields and consistently send last_issue in cap messages#60283

mds,client: correct last_issue struct fields and consistently send last_issue in cap messages#60283
batrick merged 6 commits intoceph:mainfrom
batrick:i66704

batrick commented Oct 12, 2024 •

edited

Loading

Uh oh!

batrick commented Oct 14, 2024

Uh oh!

batrick commented Oct 14, 2024

Uh oh!

batrick commented Oct 19, 2024

Uh oh!

batrick commented Oct 22, 2024

Uh oh!

vshankar commented Oct 29, 2024

Uh oh!

batrick commented Oct 31, 2024

Uh oh!

batrick commented Nov 6, 2024

Uh oh!

batrick commented Nov 13, 2024

Uh oh!

idryomov Nov 18, 2024

Uh oh!

batrick Nov 18, 2024

Uh oh!

batrick Nov 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

batrick commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

batrick commented Oct 14, 2024

Uh oh!

batrick commented Oct 14, 2024

Uh oh!

batrick commented Oct 19, 2024

Uh oh!

batrick commented Oct 22, 2024

Uh oh!

vshankar commented Oct 29, 2024

Uh oh!

batrick commented Oct 31, 2024

Uh oh!

batrick commented Nov 6, 2024

Uh oh!

batrick commented Nov 13, 2024

Uh oh!

idryomov Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

batrick Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

batrick Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

batrick commented Oct 12, 2024 •

edited

Loading