Skip to content

Commit a3e3668

Browse files
authored
[cli] Fix: recover command doesn't accept rate limit parameter (#4535)
* Fix: recover command didn't accept rate limit parameter
1 parent 175e294 commit a3e3668

File tree

2 files changed

+68
-31
lines changed

2 files changed

+68
-31
lines changed

bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ public RecoverCmd() {
524524
opts.addOption("sk", "skipOpenLedgers", false, "Skip recovering open ledgers");
525525
opts.addOption("d", "deleteCookie", false, "Delete cookie node for the bookie.");
526526
opts.addOption("sku", "skipUnrecoverableLedgers", false, "Skip unrecoverable ledgers.");
527-
opts.addOption("rate", "replicationRate", false, "Replication rate by bytes");
527+
opts.addOption("rate", "replicationRate", true, "Replication rate by bytes");
528528
}
529529

530530
@Override

bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieShellTest.java

Lines changed: 67 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import static java.nio.charset.StandardCharsets.UTF_8;
2323
import static org.junit.Assert.assertEquals;
24+
import static org.junit.Assert.assertFalse;
2425
import static org.junit.Assert.fail;
2526
import static org.mockito.ArgumentMatchers.any;
2627
import static org.mockito.ArgumentMatchers.eq;
@@ -35,6 +36,8 @@
3536
import static org.mockito.Mockito.when;
3637

3738
import com.google.common.collect.Maps;
39+
import java.io.ByteArrayOutputStream;
40+
import java.io.PrintStream;
3841
import java.util.Set;
3942
import java.util.SortedMap;
4043
import java.util.function.Function;
@@ -189,6 +192,14 @@ public void testRecoverCmdQuery() throws Exception {
189192
verify(admin, times(1)).close();
190193
}
191194

195+
@SuppressWarnings("unchecked")
196+
@Test
197+
public void testRecoverLedgerWithRateLimit() throws Exception {
198+
testRecoverCmdRecoverLedger(
199+
12345, false, false, false,
200+
"-force", "-l", "12345", "-rate", "10000", "127.0.0.1:3181");
201+
}
202+
192203
@Test
193204
public void testRecoverCmdRecoverLedgerDefault() throws Exception {
194205
// default behavior
@@ -235,21 +246,30 @@ void testRecoverCmdRecoverLedger(long ledgerId,
235246
boolean skipOpenLedgers,
236247
boolean removeCookies,
237248
String... args) throws Exception {
238-
RecoverCmd cmd = (RecoverCmd) shell.commands.get("recover");
239-
CommandLine cmdLine = parseCommandLine(cmd, args);
240-
assertEquals(0, cmd.runCmd(cmdLine));
241-
bookKeeperAdminMockedStatic.verify(() -> BookKeeperAdmin.newBookKeeperAdmin(any(ClientConfiguration.class)),
242-
times(1));
243-
verify(admin, times(1))
244-
.recoverBookieData(eq(ledgerId), any(Set.class), eq(dryrun), eq(skipOpenLedgers));
245-
verify(admin, times(1)).close();
246-
if (removeCookies) {
247-
MetadataDrivers.runFunctionWithRegistrationManager(any(ServerConfiguration.class), any(Function.class));
248-
verify(rm, times(1)).readCookie(any(BookieId.class));
249-
verify(rm, times(1)).removeCookie(any(BookieId.class), eq(version));
250-
} else {
251-
verify(rm, times(0)).readCookie(any(BookieId.class));
252-
verify(rm, times(0)).removeCookie(any(BookieId.class), eq(version));
249+
final PrintStream oldPs = System.err;
250+
try (ByteArrayOutputStream outContent = new ByteArrayOutputStream()) {
251+
System.setErr(new PrintStream(outContent));
252+
253+
RecoverCmd cmd = (RecoverCmd) shell.commands.get("recover");
254+
CommandLine cmdLine = parseCommandLine(cmd, args);
255+
assertEquals(0, cmd.runCmd(cmdLine));
256+
bookKeeperAdminMockedStatic.verify(() -> BookKeeperAdmin.newBookKeeperAdmin(any(ClientConfiguration.class)),
257+
times(1));
258+
verify(admin, times(1))
259+
.recoverBookieData(eq(ledgerId), any(Set.class), eq(dryrun), eq(skipOpenLedgers));
260+
verify(admin, times(1)).close();
261+
if (removeCookies) {
262+
MetadataDrivers.runFunctionWithRegistrationManager(any(ServerConfiguration.class), any(Function.class));
263+
verify(rm, times(1)).readCookie(any(BookieId.class));
264+
verify(rm, times(1)).removeCookie(any(BookieId.class), eq(version));
265+
} else {
266+
verify(rm, times(0)).readCookie(any(BookieId.class));
267+
verify(rm, times(0)).removeCookie(any(BookieId.class), eq(version));
268+
}
269+
assertFalse("invalid value for option detected:\n" + outContent,
270+
outContent.toString().contains("invalid value for option"));
271+
} finally {
272+
System.setErr(oldPs);
253273
}
254274
}
255275

@@ -261,6 +281,14 @@ public void testRecoverCmdRecoverDefault() throws Exception {
261281
"-force", "127.0.0.1:3181");
262282
}
263283

284+
@Test
285+
public void testRecoverWithRateLimit() throws Exception {
286+
// default behavior
287+
testRecoverCmdRecover(
288+
false, false, false, false,
289+
"-force", "127.0.0.1:3181");
290+
}
291+
264292
@Test
265293
public void testRecoverCmdRecoverDeleteCookie() throws Exception {
266294
// dryrun
@@ -307,21 +335,30 @@ void testRecoverCmdRecover(boolean dryrun,
307335
boolean removeCookies,
308336
boolean skipUnrecoverableLedgers,
309337
String... args) throws Exception {
310-
RecoverCmd cmd = (RecoverCmd) shell.commands.get("recover");
311-
CommandLine cmdLine = parseCommandLine(cmd, args);
312-
assertEquals(0, cmd.runCmd(cmdLine));
313-
bookKeeperAdminMockedStatic.verify(() -> BookKeeperAdmin.newBookKeeperAdmin(any(ClientConfiguration.class)),
314-
times(1));
315-
verify(admin, times(1))
316-
.recoverBookieData(any(Set.class), eq(dryrun), eq(skipOpenLedgers), eq(skipUnrecoverableLedgers));
317-
verify(admin, times(1)).close();
318-
if (removeCookies) {
319-
MetadataDrivers.runFunctionWithRegistrationManager(any(ServerConfiguration.class), any(Function.class));
320-
verify(rm, times(1)).readCookie(any(BookieId.class));
321-
verify(rm, times(1)).removeCookie(any(BookieId.class), eq(version));
322-
} else {
323-
verify(rm, times(0)).readCookie(any(BookieId.class));
324-
verify(rm, times(0)).removeCookie(any(BookieId.class), eq(version));
338+
final PrintStream oldPs = System.err;
339+
try (ByteArrayOutputStream outContent = new ByteArrayOutputStream()) {
340+
System.setErr(new PrintStream(outContent));
341+
342+
RecoverCmd cmd = (RecoverCmd) shell.commands.get("recover");
343+
CommandLine cmdLine = parseCommandLine(cmd, args);
344+
assertEquals(0, cmd.runCmd(cmdLine));
345+
bookKeeperAdminMockedStatic.verify(() -> BookKeeperAdmin.newBookKeeperAdmin(any(ClientConfiguration.class)),
346+
times(1));
347+
verify(admin, times(1))
348+
.recoverBookieData(any(Set.class), eq(dryrun), eq(skipOpenLedgers), eq(skipUnrecoverableLedgers));
349+
verify(admin, times(1)).close();
350+
if (removeCookies) {
351+
MetadataDrivers.runFunctionWithRegistrationManager(any(ServerConfiguration.class), any(Function.class));
352+
verify(rm, times(1)).readCookie(any(BookieId.class));
353+
verify(rm, times(1)).removeCookie(any(BookieId.class), eq(version));
354+
} else {
355+
verify(rm, times(0)).readCookie(any(BookieId.class));
356+
verify(rm, times(0)).removeCookie(any(BookieId.class), eq(version));
357+
}
358+
assertFalse("invalid value for option detected:\n" + outContent,
359+
outContent.toString().contains("invalid value for option"));
360+
} finally {
361+
System.setErr(oldPs);
325362
}
326363
}
327364

0 commit comments

Comments
 (0)