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

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

@StevenLuMT StevenLuMT requested a review from hangc0276 November 18, 2024 00:13
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

@hezhangjian
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

@@ -182,7 +182,7 @@ extraServerComponents=
# to read-only mode and serve only read requests. When all disks recovered,
# the bookie will be converted to read-write mode.Otherwise it will obey the `readOnlyModeEnabled` behavior.
# By default this will be disabled.
Copy link
Member

@StevenLuMT StevenLuMT Nov 30, 2024

Choose a reason for hiding this comment

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

By default this will be enable.

@StevenLuMT StevenLuMT merged commit b372e90 into apache:master Dec 2, 2024
23 checks passed
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