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.

Sign up for GitHub

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

Jump to bottom

mds: don't stall the asok thread for flush commands #57274

Merged
merged 2 commits into from
May 16, 2024
Merged

mds: don't stall the asok thread for flush commands #57274

merged 2 commits into from
May 16, 2024

Conversation

leonid-s-usov
Copy link
Contributor

@leonid-s-usov leonid-s-usov commented May 5, 2024 •
edited

Also, relax some timing requirements in the quiescer

Fixes: https://tracker.ceph.com/issues/65803

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

@leonid-s-usov leonid-s-usov requested a review from batrick May 5, 2024 21:22
@github-actions github-actions bot added the tests label May 5, 2024
@leonid-s-usov
Copy link
Contributor Author

This is used as the suite branch for https://pulpito.ceph.com/leonidus-2024-05-05_21:28:36-fs-wip-pdonnell-testing-20240503.163550-debug-distro-default-smithi/

@leonid-s-usov leonid-s-usov requested a review from a team May 6, 2024 06:20
vshankar
vshankar reviewed May 6, 2024
View reviewed changes
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.

Looks fine!

Conditional approve post successful test run.

batrick
batrick requested changes May 6, 2024
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

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

I think the simple fix is to make flush journal asynchronous. The quiesce command already is so please just do some similar logic there.

@leonid-s-usov
Copy link
Contributor Author

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

I think the simple fix is to make flush journal asynchronous. The quiesce command already is so please just do some similar logic there.

I'm not sure it's that simple. We can't just change a synchronous command to asynchronous, can we? We'll have to add a switch to the command and change the invocation along with verifying that it'll know how to deal with the new approach.

In any case, it would probably have to be a separate ticket with its own PR...

@github-actions github-actions bot added the cephfs Ceph File System label May 6, 2024
@leonid-s-usov leonid-s-usov changed the title qa/quiescer: relax timing requirements for teuthology mds: don't stall the asok thread for flush commands May 6, 2024
@leonid-s-usov
Copy link
Contributor Author

https://tracker.ceph.com/issues/65803:#note-2
I think the simple fix is to make flush journal asynchronous. The quiesce command already is so please just do some similar logic there.

I'm not sure it's that simple. We can't just change a synchronous command to asynchronous, can we? We'll have to add a switch to the command and change the invocation along with verifying that it'll know how to deal with the new approach.

In any case, it would probably have to be a separate ticket with its own PR...

In a talk with @batrick he explained that it was about not holding up the asok thread, which I followed here

@leonid-s-usov leonid-s-usov requested a review from batrick May 6, 2024 17:03
@leonid-s-usov
Copy link
Contributor Author

jenkins test make check

@leonid-s-usov
Copy link
Contributor Author

jenkins test make check

@batrick
Copy link
Member

batrick commented May 7, 2024

jenkins test make check arm64

vshankar
vshankar requested changes May 8, 2024
View reviewed changes
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.

Nicely done.

vshankar
vshankar approved these changes May 8, 2024
View reviewed changes
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.

Approved!

@batrick
Copy link
Member

batrick commented May 8, 2024

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

@leonid-s-usov leonid-s-usov mentioned this pull request May 14, 2024
Merged
@leonid-s-usov
Copy link
Contributor Author

@batrick please approve for merge. See #57332 (comment)

@batrick batrick merged commit 70ed382 into main May 16, 2024
11 checks passed
@batrick batrick deleted the wip-lusov-quiescer-relax branch May 16, 2024 02:57
This was referenced May 20, 2024
Closed
Open
@leonid-s-usov leonid-s-usov mentioned this pull request May 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@batrick batrick batrick approved these changes

@vshankar vshankar vshankar approved these changes

@lxbsz lxbsz lxbsz approved these changes

Assignees
No one assigned
Labels
cephfs Ceph File System needs-qa tests wip-pdonnell-testing
Projects
None yet
Milestone
No milestone
4 participants
@leonid-s-usov @batrick @vshankar @lxbsz