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

[improve][conf] Change the default value of readOnlyModeOnAnyDiskFullEnabled to true #4520

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

liangyepianzhou
Copy link
Contributor

@liangyepianzhou liangyepianzhou commented Oct 31, 2024

Motivation

The #3212 import a feature: convert bookie to read-only mode when any ledger disks is full.
In our practice, we found that when the bookie disk is full, if it is not changed to Read Only mode, a large number of messages will fail to be written to the ledger disk. And because the bookie status is still available at this time, no alarm is given to the maintenance personnel, resulting in slow access for the maintenance personnel.

That feature has been in place for some time. It is safe to enable it default.

PS: It was mentioned in the PR discussion that an email discussion should be submitted, but no follow-up was done. [0]
[0] - https://github.com/apache/bookkeeper/pull/3212/files#r857122376

Changes

Change the default value of readOnlyModeOnAnyDiskFullEnabled to true

@eolivelli
Copy link
Contributor

I agree with this change but I cannot find the discussion on the mailing list

@liangyepianzhou
Copy link
Contributor Author

I agree with this change but I cannot find the discussion on the mailing list

Thank you for your support. I apologize for not starting the discussion on the mailing list earlier due to my busy schedule. I have just sent out the discussion email, and you can find it here: https://lists.apache.org/thread/cvspxb4d1ryy4wo1j2k5r1qr9fcjsd42

Copy link
Member

@StevenLuMT StevenLuMT left a comment

Choose a reason for hiding this comment

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

LGTM

@shoothzj
Copy link
Member

@lhotari currently, is there any labels to track release note updates?

@lhotari lhotari added the release/important-notice This change should be mentioned in release notes. label Nov 27, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari
Copy link
Member

lhotari commented Nov 27, 2024

@lhotari currently, is there any labels to track release note updates?

@shoothzj good point. I added a label release/important-notice for that purpose. It's something that exists in Pulsar too.

@StevenLuMT
Copy link
Member

StevenLuMT commented Nov 28, 2024

image

https://github.com/apache/bookkeeper/actions/runs/11605956158/job/33595453553?pr=4520
please update the testcase : org.apache.bookkeeper.bookie.LedgerDirsManagerTest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config release/important-notice This change should be mentioned in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants