Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mds: just wait the client flushes the snap and dirty buffer #53238

Merged
merged 1 commit into from Jan 18, 2024

Conversation

lxbsz
Copy link
Member

@lxbsz lxbsz commented Aug 31, 2023

When truncating the inode we will just set the ifile lock state to LOCK_XLOCKSNAP and then try to revoke the 'Fb' caps, but if the client couldn't release the 'Fb' cap in time just replies with a normal cap updating request, the MDS will successfully transfer the ifile's lock state to LOCK_EXCL, which is stable.

That means the MDS will wake up the truncating request and continue truncating the objects from Rados without waiting the clients to flush the diry buffer.

Fixes: commit 9c65920 ("mds: force client flush snap data before
truncating objects")
Fixes: https://tracker.ceph.com/issues/62580

Contribution Guidelines

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

@lxbsz lxbsz requested review from vshankar, batrick and a team August 31, 2023 09:40
@github-actions github-actions bot added the cephfs Ceph File System label Aug 31, 2023
@vshankar
Copy link
Contributor

jenkins test windows

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

Fix makes sense.

@vshankar
Copy link
Contributor

jenkins test windows

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

@vshankar
Copy link
Contributor

jenkins test windows

1 similar comment
@vshankar
Copy link
Contributor

jenkins test windows

@vshankar
Copy link
Contributor

@lxbsz could you please rebase and push - jenkins test are failing to pass :/

@lxbsz
Copy link
Member Author

lxbsz commented Oct 23, 2023

@lxbsz could you please rebase and push - jenkins test are failing to pass :/

Done.

@lxbsz
Copy link
Member Author

lxbsz commented Oct 23, 2023

jenkins test make check arm64

@vshankar
Copy link
Contributor

vshankar commented Nov 2, 2023

jenkins test windows

@vshankar
Copy link
Contributor

vshankar commented Nov 6, 2023

@lxbsz please rebase and push - jenkins mess :/

@lxbsz
Copy link
Member Author

lxbsz commented Nov 7, 2023

@lxbsz please rebase and push - jenkins mess :/

Done.

@lxbsz
Copy link
Member Author

lxbsz commented Nov 7, 2023

jenkins test make check arm64

@vshankar
Copy link
Contributor

vshankar commented Nov 7, 2023

Windows failure looks real

[2023-11-07T03:02:11.000Z] [isolated][googletest] ceph_test_libcephfs failed. Error: Command timed out (1800s): "cmd /c 'C:\ceph\ceph_test_libcephfs.exe --gtest_output=xml:C:\workspace\test_results\out\ceph_test_libcephfs\ceph_test_libcephfs_results.xml --gtest_filter="-LibCephFS.Deleg*" >> C:\workspace\test_results\out\ceph_test_libcephfs\ceph_test_libcephfs_results.log 2>&1'".

@petrutlucian94 @lxbsz

@petrutlucian94
Copy link
Contributor

Windows failure looks real

[2023-11-07T03:02:11.000Z] [isolated][googletest] ceph_test_libcephfs failed. Error: Command timed out (1800s): "cmd /c 'C:\ceph\ceph_test_libcephfs.exe --gtest_output=xml:C:\workspace\test_results\out\ceph_test_libcephfs\ceph_test_libcephfs_results.xml --gtest_filter="-LibCephFS.Deleg*" >> C:\workspace\test_results\out\ceph_test_libcephfs\ceph_test_libcephfs_results.log 2>&1'".

@petrutlucian94 @lxbsz

LibCephFS.SnapDiffLib2 seems to hang, I'll try running it locally.

@petrutlucian94
Copy link
Contributor

It's not a Windows issue, it hangs on Linux as well. It's just that make check doesn't run ceph_test_libcephfs. Here's a trace:

Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
bt
__futex_abstimed_wait_common64 (private=0, cancel=true, abstime=0x0, op=393, expected=0, futex_word=0x7fff89826648) at ./nptl/futex-internal.c:57
57	./nptl/futex-internal.c: No such file or directory.
(gdb) bt
#0  __futex_abstimed_wait_common64 (private=0, cancel=true, abstime=0x0, op=393, expected=0, 
    futex_word=0x7fff89826648) at ./nptl/futex-internal.c:57
#1  __futex_abstimed_wait_common (cancel=true, private=0, abstime=0x0, clockid=0, expected=0, 
    futex_word=0x7fff89826648) at ./nptl/futex-internal.c:87
#2  __GI___futex_abstimed_wait_cancelable64 (futex_word=futex_word@entry=0x7fff89826648, 
    expected=expected@entry=0, clockid=clockid@entry=0, abstime=abstime@entry=0x0, 
    private=private@entry=0) at ./nptl/futex-internal.c:139
#3  0x00007f5739384a41 in __pthread_cond_wait_common (abstime=0x0, clockid=0, mutex=0x561a234fe950, 
    cond=0x7fff89826620) at ./nptl/pthread_cond_wait.c:503
#4  ___pthread_cond_wait (cond=0x7fff89826620, mutex=0x561a234fe950) at ./nptl/pthread_cond_wait.c:627
#5  0x00007f573a4f7e7d in std::condition_variable::wait<Client::make_request(MetaRequest*, const UserPerm&, InodeRef*, bool*, mds_rank_t, ceph::bufferlist*, size_t)::<lambda()> > (__p=..., __lock=..., 
    this=0x7fff89826620) at /usr/include/c++/11/condition_variable:103
#6  Client::make_request (this=<optimized out>, request=0x561a235c72b0, perms=..., ptarget=0x0, 
    pcreated=0x0, use_mds=<optimized out>, pdirbl=0x0, feature_needed=18446744073709551615)
    at /home/ubuntu/workspace/ceph.linux/src/client/Client.cc:2053
#7  0x00007f573a514fdc in Client::_do_setattr (this=0x561a234fe7d0, in=0x7f5714013310, 
    stx=<optimized out>, mask=32, perms=..., inp=0x0, aux=0x0)
    at /home/ubuntu/workspace/ceph.linux/src/client/Client.cc:8308
#8  0x00007f573a51937c in Client::__setattrx (this=0x561a234fe7d0, in=0x7f5714013310, 
    stx=0x7fff89826950, mask=32, perms=..., inp=<optimized out>)
    at /home/ubuntu/workspace/ceph.linux/src/client/Client.cc:8343
#9  0x00007f573a53e091 in Client::_setattrx (this=<optimized out>, in=..., stx=<optimized out>, 
    mask=<optimized out>, perms=...) at /home/ubuntu/workspace/ceph.linux/src/client/Client.cc:8363
#10 0x00007f573a53fa05 in Client::setattrx (this=0x561a234fe7d0, relpath=<optimized out>, 
    stx=0x7fff89826950, mask=32, perms=..., flags=0)
    at /home/ubuntu/workspace/ceph.linux/src/client/Client.cc:8423
#11 0x00007f573a53fc16 in Client::truncate (this=<optimized out>, relpath=<optimized out>, 
    length=<optimized out>, perms=...) at /home/ubuntu/workspace/ceph.linux/src/client/Client.cc:11720
#12 0x0000561a2144ae55 in TestMount::write_full (this=0x7fff89826bc0, relpath=<optimized out>, 
    data="hello world to be removed at snap3") at /usr/include/c++/11/bits/basic_string.h:194
#13 0x0000561a21437daf in TestMount::prepareSnapDiffLib2Cases (this=0x7fff89826bc0)
    at /usr/include/c++/11/ext/new_allocator.h:79
#14 0x0000561a21444c67 in LibCephFS_SnapDiffLib2_Test::TestBody (this=<optimized out>)
    at /home/ubuntu/workspace/ceph.linux/src/test/libcephfs/snapdiff.cc:817
#15 0x0000561a21480fef in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>
    (location=0x561a2149483a "the test body", method=<optimized out>, object=0x561a235c22a0)
    at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2605
#16 testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void> (
    object=object@entry=0x561a235c22a0, method=<optimized out>, 
    location=location@entry=0x561a2149483a "the test body")
    at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2641
#17 0x0000561a214756f6 in testing::Test::Run (this=0x561a235c22a0)
    at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2680
#18 testing::Test::Run (this=0x561a235c22a0)
    at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2670
#19 0x0000561a21475855 in testing::TestInfo::Run (this=0x561a23387830)
    at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2858
#20 testing::TestInfo::Run (this=0x561a23387830)
    at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2831

@lxbsz
Copy link
Member Author

lxbsz commented Nov 8, 2023

It's not a Windows issue, it hangs on Linux as well. It's just that make check doesn't run ceph_test_libcephfs. Here's a trace:

Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
bt

[...]

#20 testing::TestInfo::Run (this=0x561a23387830)
at /home/ubuntu/workspace/ceph.linux/src/googletest/googletest/src/gtest.cc:2831

@petrutlucian94 BTW, did the above test include this PR ? Or just the
upstream ?

@lxbsz
Copy link
Member Author

lxbsz commented Nov 8, 2023

jenkins test make check arm64

@petrutlucian94
Copy link
Contributor

The above test fails if I apply this commit but passes if I revert it, so there's something wrong with the commit.

./ceph_test_libcephfs --gtest_filter="*SnapDiffLib2*"
Note: Google Test filter = *SnapDiffLib2*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from LibCephFS
[ RUN      ] LibCephFS.SnapDiffLib2
2 vs. 3 vs. 4
---------snap1 listing verification---------
...
------------- closing -------------
[       OK ] LibCephFS.SnapDiffLib2 (11042 ms)
[----------] 1 test from LibCephFS (11042 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (11042 ms total)
[  PASSED  ] 1 test.
$ git log -5 --oneline
3a1ad72220e (HEAD) Revert "mds: just wait the client flushes the snap and dirty buffer"
034512a6e5c (lxbsz/wip-62580) mds: just wait the client flushes the snap and dirty buffer
f76333be1a2 Merge pull request #54197 from ivancich/wip-bucket-stats-add-gen
125cc9406ab Merge pull request #54365 from cbodley/wip-63455
c6b83286033 Merge PR #53568 into main

@lxbsz
Copy link
Member Author

lxbsz commented Nov 8, 2023

The above test fails if I apply this commit but passes if I revert it, so there's something wrong with the commit.

./ceph_test_libcephfs --gtest_filter="*SnapDiffLib2*"
Note: Google Test filter = *SnapDiffLib2*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from LibCephFS
[ RUN      ] LibCephFS.SnapDiffLib2
2 vs. 3 vs. 4
---------snap1 listing verification---------
...
------------- closing -------------
[       OK ] LibCephFS.SnapDiffLib2 (11042 ms)
[----------] 1 test from LibCephFS (11042 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (11042 ms total)
[  PASSED  ] 1 test.
$ git log -5 --oneline
3a1ad72220e (HEAD) Revert "mds: just wait the client flushes the snap and dirty buffer"
034512a6e5c (lxbsz/wip-62580) mds: just wait the client flushes the snap and dirty buffer
f76333be1a2 Merge pull request #54197 from ivancich/wip-bucket-stats-add-gen
125cc9406ab Merge pull request #54365 from cbodley/wip-63455
c6b83286033 Merge PR #53568 into main

Okay, I will have a look later. Thanks @petrutlucian94

When truncating the inode we will just set the ifile lock state to
LOCK_XLOCKSNAP and then try to revoke the 'Fb' caps, but if the
client couldn't release the 'Fb' cap in time just replies with a
normal cap updating request, the MDS will successfully transfer
the ifile's lock state to LOCK_EXCL, which is stable.

That means the MDS will wake up the truncating request and continue
truncating the objects from Rados without waiting the clients to
flush the diry buffer.

Fixes: commit 9c65920 ("mds: force client flush snap data before
		truncating objects")
Fixes: https://tracker.ceph.com/issues/62580
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
Copy link
Member Author

lxbsz commented Nov 15, 2023

Fixed it. @vshankar Please review it again, thanks!

@vshankar
Copy link
Contributor

Fixed it. @vshankar Please review it again, thanks!

I've picked it up for fs suite runs @lxbsz. What was the cause of the failure BTW?

@lxbsz
Copy link
Member Author

lxbsz commented Nov 22, 2023

Fixed it. @vshankar Please review it again, thanks!

I've picked it up for fs suite runs @lxbsz. What was the cause of the failure BTW?

The old code couldn't correctly handle the corner cases and couldn't wake up the waiters. The new fixes just make it more precise and only do this just in case in LOCK_XLOCKSNAP state.

@vshankar
Copy link
Contributor

Test runs in ~2h - wip-vshankar-testing-20231127.102654

@vshankar
Copy link
Contributor

vshankar commented Dec 4, 2023

https://pulpito.ceph.com/?branch=wip-vshankar-testing-20231127.102654

(rhel pkg install failures are a bunch, so, this would need a revalidate)

@vshankar
Copy link
Contributor

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cephfs Ceph File System
Projects
None yet
3 participants