client: fire the finish_cap_snap() after buffer being flushed by lxbsz · Pull Request #38732 · ceph/ceph

lxbsz · 2020-12-30T01:45:50Z

When the inode has Fb cap and the used reference is none zero, the
cap snap flushing will be delayed, so we need to make sure it will
be invoked after the dirty buffer is flushed to osd.

Fixes: https://tracker.ceph.com/issues/48679
Signed-off-by: Xiubo Li xiubli@redhat.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

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 api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

lxbsz · 2021-01-04T03:29:02Z

jenkins test make check

batrick

Otherwise LGTM. Thanks for tracking that down!

src/client/Client.cc

lxbsz · 2021-01-07T02:32:37Z

jenkins test api

batrick · 2021-01-08T23:11:20Z

https://pulpito.ceph.com/pdonnell-2021-01-08_05:07:01-fs-wip-pdonnell-testing-20210107.220115-distro-basic-smithi/

New failures are:

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

New failures are related to snaps. I fear this PR may be the cause.

Signed-off-by: Xiubo Li <xiubli@redhat.com>

When the inode has Fb cap and the used reference is none zero, the cap snap flushing will be delayed, so we need to make sure it will be invoked after the dirty buffer is flushed to osd. Fixes: https://tracker.ceph.com/issues/48679 Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz · 2021-01-09T01:51:11Z

https://pulpito.ceph.com/pdonnell-2021-01-08_05:07:01-fs-wip-pdonnell-testing-20210107.220115-distro-basic-smithi/

New failures are:

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

New failures are related to snaps. I fear this PR may be the cause.

I think I fogot to clean the writing flag and wakeup the waiters.

lxbsz · 2021-01-09T01:51:51Z

@batrick Fixed it, PTAL. Thanks.

batrick · 2021-01-09T17:55:21Z

https://pulpito.ceph.com/?branch=wip-pdonnell-testing-20210109.031116

master with same suites:

https://pulpito.ceph.com/pdonnell-2021-01-09_22:14:16-fs:snaps-master-distro-basic-smithi/
https://pulpito.ceph.com/pdonnell-2021-01-09_22:14:52-fs:functional-master-distro-basic-smithi/
https://pulpito.ceph.com/pdonnell-2021-01-09_22:15:16-fs:thrash-master-distro-basic-smithi/

lxbsz · 2021-02-02T02:31:20Z

https://pulpito.ceph.com/?branch=wip-pdonnell-testing-20210109.031116

master with same suites:

https://pulpito.ceph.com/pdonnell-2021-01-09_22:14:16-fs:snaps-master-distro-basic-smithi/
https://pulpito.ceph.com/pdonnell-2021-01-09_22:14:52-fs:functional-master-distro-basic-smithi/
https://pulpito.ceph.com/pdonnell-2021-01-09_22:15:16-fs:thrash-master-distro-basic-smithi/

@batrick
Checked the failure ones, almost all are kclient related and the the failures are the same with master branch. This PR won't affect the kclient mounters.

lxbsz · 2021-03-24T03:16:47Z

@batrick

The new issue https://tracker.ceph.com/issues/49928 is also caused by this bug.

I think the old code want to call the check_cap(), which will also trigger to flush the cap snap, in put_cap_ref() when putting the last ref for the Fb cap, but it didn't. This PR will explicitly do that.

batrick · 2021-03-24T16:09:22Z

ACK, thanks for checking htat

batrick · 2021-03-26T22:10:48Z

https://pulpito.ceph.com/pdonnell-2021-03-24_23:26:35-fs-wip-pdonnell-testing-20210324.190252-distro-basic-smithi/

https://tracker.ceph.com/issues/49500
qa: "Assertion `cb_done' failed."
https://tracker.ceph.com/issues/50019
qa: mount failure with cephadm "probably no MDS server is up?"
https://tracker.ceph.com/issues/50020
qa: "RADOS object not found (Failed to operate read op for oid cephfs_mirror)"
https://tracker.ceph.com/issues/48773
qa: scrub does not complete
https://tracker.ceph.com/issues/45434
qa: test_full_fsync (tasks.cephfs.test_full.TestClusterFull) failed
https://tracker.ceph.com/issues/48805
mds: "cluster [WRN] Scrub error on inode 0x1000000039d (/client.0/tmp/blogbench-1.0/src/blogtest_in) see mds.a log and damage ls output for details"
https://tracker.ceph.com/issues/48772
qa: pjd: not ok 9, 44, 80
https://tracker.ceph.com/issues/50021
qa: snaptest-git-ceph failure during mon thrashing
https://tracker.ceph.com/issues/48771
qa: iogen: workload fails to cause balancing
https://tracker.ceph.com/issues/50016
qa: test_damage: "RuntimeError: 2 mutations had unexpected outcomes"
https://tracker.ceph.com/issues/49466
qa: "Command failed on gibba030 with status 1: 'set -ex\nsudo dd of=/tmp/tmp.ZEeZBasJer'"

batrick · 2021-03-26T22:14:33Z

This change looks good/safe to me so I'm merging it. We did have this new failure come up but I don't think it's related, so far:

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

Please have a look though.

lxbsz · 2021-03-27T03:21:12Z

This change looks good/safe to me so I'm merging it. We did have this new failure come up but I don't think it's related, so far:

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

Please have a look though.

Sure, will check it.

lxbsz requested a review from a team December 30, 2020 01:46

github-actions bot added the cephfs Ceph File System label Dec 30, 2020

lxbsz requested a review from batrick December 30, 2020 01:46

lxbsz added the bug-fix label Dec 30, 2020

lxbsz force-pushed the client_cap_snaps branch from 09008c8 to 859c364 Compare December 30, 2020 09:22

batrick requested changes Jan 4, 2021

View reviewed changes

src/client/Client.cc Outdated Show resolved Hide resolved

lxbsz requested a review from batrick January 4, 2021 23:59

lxbsz force-pushed the client_cap_snaps branch 2 times, most recently from a6d3d64 to 616084c Compare January 7, 2021 01:28

batrick approved these changes Jan 7, 2021

View reviewed changes

batrick added needs-qa wip-pdonnell-testing labels Jan 7, 2021

batrick added wip-pdonnell-testing2 and removed needs-qa wip-pdonnell-testing labels Jan 8, 2021

lxbsz added 4 commits January 9, 2021 09:24

client: clean up the code

820d39d

Signed-off-by: Xiubo Li <xiubli@redhat.com>

client: remove unused _flushed_cap_snap

f60329b

Signed-off-by: Xiubo Li <xiubli@redhat.com>

client: simplify the iterating code

1bea6e5

Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz force-pushed the client_cap_snaps branch from 616084c to a34d0c2 Compare January 9, 2021 01:50

lxbsz requested a review from batrick January 9, 2021 01:52

batrick approved these changes Jan 9, 2021

View reviewed changes

batrick added needs-qa wip-pdonnell-testing labels Jan 9, 2021

batrick removed wip-pdonnell-testing wip-pdonnell-testing2 needs-qa labels Jan 9, 2021

lxbsz requested a review from batrick February 2, 2021 02:31

batrick added the needs-review label Feb 18, 2021

batrick added needs-qa and removed needs-review labels Mar 24, 2021

batrick added the wip-pdonnell-testing label Mar 24, 2021

batrick merged commit 8a4794c into ceph:master Mar 26, 2021

This was referenced Apr 9, 2021

nautilus: client: fire the finish_cap_snap() after buffer being flushed #40722

Merged

octopus: client: fire the finish_cap_snap() after buffer being flushed #40778

Merged

Conversation

lxbsz commented Dec 30, 2020

Checklist

Uh oh!

lxbsz commented Jan 4, 2021

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lxbsz commented Jan 7, 2021

Uh oh!

batrick commented Jan 8, 2021

Uh oh!

lxbsz commented Jan 9, 2021

Uh oh!

lxbsz commented Jan 9, 2021

Uh oh!

batrick commented Jan 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz commented Feb 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz commented Mar 24, 2021

Uh oh!

batrick commented Mar 24, 2021

Uh oh!

batrick commented Mar 26, 2021

Uh oh!

batrick commented Mar 26, 2021

Uh oh!

lxbsz commented Mar 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

batrick commented Jan 9, 2021 •

edited

Loading

lxbsz commented Feb 2, 2021 •

edited

Loading