Skip to content

Support markLedgerReplicated command.#4032

Open
horizonzy wants to merge 7 commits intoapache:masterfrom
horizonzy:support-markLedgerReplicated-command
Open

Support markLedgerReplicated command.#4032
horizonzy wants to merge 7 commits intoapache:masterfrom
horizonzy:support-markLedgerReplicated-command

Conversation

@horizonzy
Copy link
Member

@horizonzy horizonzy commented Jul 17, 2023

Support markLedgerReplicated command.

Motivation:
Sometimes, users need to decommission a bookie, but there are under-replicated ledgers on the bookie that would be blocked during decommission. When users don't care about the under-replicated data, they hope to mark the ledger directly as replicated so that the decommission can proceed smoothly.

@hangc0276
Copy link
Contributor

@horizonzy Would you please add more background of adding this command?

@horizonzy horizonzy closed this Jul 18, 2023
@horizonzy horizonzy reopened this Jul 18, 2023
@horizonzy
Copy link
Member Author

@horizonzy Would you please add more background of adding this command?

Done


MarkLedgerReplicatedCmd() {
super(CMD_MARKLEDGERREPLICATED);
opts.addOption("l", "ledgerid", true, "Ledger ID");
Copy link
Member

Choose a reason for hiding this comment

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

It's better to provide a list or range option to allow given a group of ledger id to mark. For example, we can use , to give a list of ledgers, "1,2,3,4,5,6". We also can use a range to specify the ledger, "1...10".
This can avoid calling this command multiple times when there has many ledgers that need to be marked.

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

Please add the documents for the newly added command. Please refer to https://bookkeeper.apache.org/docs/reference/cli#bookkeeper-shell


MarkLedgerReplicatedCmd() {
super(CMD_MARKLEDGERREPLICATED);
opts.addOption("l", "ledgerid", true, "Ledger ID");
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

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.

4 participants