Add a REST API to get or update bookie readOnly state

[fantapsody]

Motivation

This PR is a part of the work to improve the process of removing bookies from the cluster. Specifically, it implements the readOnly API described in the mail.

Changes

• Add an REST API at /api/v1/bookie/state/readonly
  • The GET method returns the current readOnly status
  • The PUT method updates the readOnly status if needed.

TODOs

• Update the document once the PR is accepted.
• Update the BookieStateManager & BookieImpl to persist the information that the state change is triggered by the external API request and do not change the state based on the notification from the dirs monitoring service.

[fantapsody]

@ivankelly PTAL

[eolivelli]

what happens if you restart the bookie ?

[fantapsody]

what happens if you restart the bookie ?

@eolivelli It depends on the value of the config persistBookieStatusEnabled. If it is set to true, the state will be persisted to disk and be restored after restarted. However, the state might still be changed by the LedgerDirsMonitor, and I plan to persist the reason for the state change and use that information to decide whether the state could be changed by LedgerDirsMonitor. That may require the change of how to save & load the bookie status so I plan to leave it to further PRs.

[eolivelli]

makes sense to me

the only thing to add here now is a bunch of test cases that cover this new code

well done

[fantapsody]

It seems there is no test on HTTP services yet, so I plan to add one in the integration test module. I tried to run some existing tests like SimpleClusterTest in tests::integration::cluster to see how it works, but the test cluster failed to start and showed error logs like

Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for URL to be accessible (http://localhost:55041/commands/ruok should return HTTP 200)

I checked the log of the action that ran the integration tests, it seems those tests were not run actually: https://github.com/apache/bookkeeper/pull/2799/checks?check_run_id=3650019520

@eolivelli Could you confirm those integration tests are all still valid? Or do you have any suggestions on how to test these HTTP services? Thanks!

[eolivelli]

I cannot check now.
But IIRC we have unit tests for the HTTP API.
No need for a full integration test

In theory integration tests run for every pr validation

[eolivelli]

See
https://github.com/apache/bookkeeper/pull/2799/checks?check_run_id=3650019520

[fantapsody]

I cannot check now.
But IIRC we have unit tests for the HTTP API.
No need for a full integration test

In theory integration tests run for every pr validation

I just found those tests and will work on a new test case soon, thanks!

[fantapsody]

@eolivelli I added a unit test for the new API, please take another look, thanks!

[addisonj]

@sijie Can you review this as well?

[pradeepbn]

@fantapsody @sijie @eolivelli can we get this merged?

[letterwuyu]

thank you for you great work，lgtm， any updates ?

[lujiwen]

@fantapsody thanks a lot for your great work, I am also looking for the solution to gracefully remove a bookie from cluster by marking it as read only mode, and do not handle write request any longer. But my question is how pulsar broker does realize that a read-only bookie is not a candidate of ensemble, and does not send write request to? Thanks in advance!

[fantapsody]

@lujiwen The selection of bookies for the quorum is determined by the bookkeeper client which watches the changes of bookies and selects active bookies based on configurations and predefined policies. You may find a detailed description in https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/EnsemblePlacementPolicy.java. This PR is just a start, and there would be follow-ups as you may see in the TODOs in the issue description and the discussion in the mail list.

@eolivelli @sijie Could you take a look at the PR again?

[lujiwen]

@fantapsody Thank you!

[eolivelli]

Lgtm