Skip to content

mgr: return EAGAIN when ceph-mgr request queue is full#60194

Closed
neesingh-rh wants to merge 1 commit intoceph:mainfrom
neesingh-rh:wip-62712
Closed

mgr: return EAGAIN when ceph-mgr request queue is full#60194
neesingh-rh wants to merge 1 commit intoceph:mainfrom
neesingh-rh:wip-62712

Conversation

@neesingh-rh
Copy link
Contributor

@neesingh-rh neesingh-rh commented Oct 8, 2024

Fixes: https://tracker.ceph.com/issues/62712
Signed-off-by: Neeraj Pratap Singh neesingh@redhat.com

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

@neesingh-rh neesingh-rh added the cephfs Ceph File System label Oct 8, 2024
@neesingh-rh neesingh-rh requested a review from a team October 8, 2024 12:02
@neesingh-rh neesingh-rh requested a review from a team as a code owner October 8, 2024 12:02
@neesingh-rh
Copy link
Contributor Author

neesingh-rh commented Oct 8, 2024

This PR is open for any suggestions w.r.t. the naming of functions/commands too along with code reviews. I have used what looked apt to me.

@vshankar vshankar changed the title src: implement EAGAIN logic for clearing request queue when under… mgr: return EAGAIN when ceph-mgr request queue is full Oct 8, 2024
@vshankar vshankar requested a review from a team October 8, 2024 13:36
@neesingh-rh
Copy link
Contributor Author

jenkins retest this please

// Optional, URI exposed by plugins that implement serve()
std::string uri;

std::atomic_uint mod_finisher_cnt{0};
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this more, let's move this to the Finisher class to manage the number of contexts "to be completed". (They may be in the in_progress_queue or finisher_queue.)

Copy link
Member

Choose a reason for hiding this comment

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

Make sure to add the count to the perf dump output for the Finisher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied the suggestions, PTAL

Comment on lines +2559 to +2574
auto finisher_val = py_modules.get_threshold_val(mod_name);
auto queue_len = py_modules.get_mod_finisher_queue_len(mod_name);
ss << "The value of 'max_command_queue_length' config option for Module '" << mod_name << "' is "
<< finisher_val << " and the current request queue length is " << queue_len;
dout(10) << ss.str() << dendl;

if (finisher_val != "0") {
if (std::to_string(queue_len) > finisher_val)
{
ss << "Module '" << mod_name << "' has passed its finisher thread limit. "
"Set the threshold limit using the 'max_command_queue_length' config ";
dout(4) << ss.str() << dendl;
cmdctx->reply(-EAGAIN, ss);
return true;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this part looks okay otherwise

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added stale and removed stale labels Mar 17, 2025
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

… load

Fixes: https://tracker.ceph.com/issues/62712
Signed-off-by: Neeraj Pratap Singh <neesingh@redhat.com>
@batrick
Copy link
Member

batrick commented Jun 13, 2025

STatus on this?

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Aug 12, 2025
@vshankar vshankar removed the stale label Sep 9, 2025
@github-actions
Copy link

github-actions bot commented Nov 8, 2025

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Nov 8, 2025
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Dec 8, 2025
@github-project-automation github-project-automation bot moved this from Review in progress to Done in Ceph-Dashboard Dec 8, 2025
@neesingh-rh neesingh-rh reopened this Dec 15, 2025
@github-project-automation github-project-automation bot moved this from Done to New in Ceph-Dashboard Dec 15, 2025
@github-actions github-actions bot removed the stale label Dec 15, 2025
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Feb 13, 2026
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Mar 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants