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

pacific: log: Make log_max_recent have an effect again. #48311

Merged
merged 1 commit into from Jan 24, 2024

Conversation

k0ste
Copy link
Contributor

@k0ste k0ste commented Sep 30, 2022

backport tracker: https://tracker.ceph.com/issues/56635


backport of #46736
parent tracker: https://tracker.ceph.com/issues/56093

this backport was staged using ceph-backport.sh version 16.0.0.6848
find the latest version at https://github.com/ceph/ceph/blob/main/src/script/ceph-backport.sh

@github-actions github-actions bot added this to the pacific milestone Sep 30, 2022
@k0ste
Copy link
Contributor Author

k0ste commented Jan 24, 2023

needs-qa

@ljflores
Copy link
Contributor

ljflores commented Jan 10, 2024

@cbodley @k0ste @idryomov is this a blocker for 16.2.15? If so, add it to https://github.com/ceph/ceph/milestone/17 since Pacific is going EOL.

@k0ste
Copy link
Contributor Author

k0ste commented Jan 10, 2024

@cbodley @k0ste @idryomov is this a blocker for 16.2.15? If so, add it to https://github.com/ceph/ceph/milestone/17 since Pacific is going EOL.

Only developers can edit labels and milestones, I'm not

Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

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

Conflicts:

  • file: src/common/options/global.yaml.in
    desc: not exists in pacific?

This file indeed doesn't exist in pacific, but simply dropping the hunk isn't right. Please tack on .set_min(1) on log_max_recent entry in src/common/options.cc:

    Option("log_max_recent", Option::TYPE_INT, Option::LEVEL_ADVANCED)
    .set_default(500)
    .set_daemon_default(10000)
    .set_min(1)
    .set_description("recent log entries to keep in memory to dump in the event of a crash")

@idryomov idryomov modified the milestones: pacific, v16.2.15 Jan 11, 2024
The log improvements in a747aea
unfortunately left log_max_recent broken because m_max_recent wasn't
used anymore.

Eliminate m_max_recent and set the capacity of the m_recent ring buffer
when log_max_recent changes. In order to call set_capacity(),
ConcreteEntry needed its move constructor set noexcept.

I haven't followed the boost code all the way down but I suspect that
setting the ring buffer capacity to anything less than 1 entry will
probably cause problems, so restrict log_max_recent to >=1.

Also fix a wrong variable used for printing the max new entries during
"log dump".

Signed-off-by: Joshua Baergen <jbaergen@digitalocean.com>
(cherry picked from commit 3d59ba1)

Conflicts:
  - file: src/common/options/global.yaml.in
    desc: file not exists in pacific
  - file: src/common/options.cc
    desc: added 'set_min' for log_max_recent
@k0ste
Copy link
Contributor Author

k0ste commented Jan 12, 2024

Ilya, done

Copy link
Contributor

@mchangir mchangir left a comment

Choose a reason for hiding this comment

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

@yuriw yuriw merged commit f7a9526 into ceph:pacific Jan 24, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants