You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/09/13 15:21:09 UTC

[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3457: BP-56: Support data migration

hangc0276 commented on code in PR #3457:
URL: https://github.com/apache/bookkeeper/pull/3457#discussion_r969747288


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;

Review Comment:
   Use `List<String>` instead of "," separate String.



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java:
##########
@@ -1951,6 +1953,56 @@ public int runCmd(CommandLine cmdLine) throws Exception {
         }
     }
 
+    /**
+     * Migrate the specified ledger data on some bookie to other bookies.
+     * */
+    class ReplicasMigrationCmd extends MyCommand {
+
+        ReplicasMigrationCmd() {
+            super(CMD_REPLICASMIGRATION);
+            opts.addOption("b", "bookieIds", true,
+                    "Bookies corresponding to the migrated replica");
+            opts.addOption("l", "ledgerIds", true,
+                    "The ledgerIds corresponding to the migrated replica");
+            opts.addOption("r", "readOnly", true,

Review Comment:
   1. required = false?
   2. We'd better set the default readOnly flag to `false`. If the admin uses this command to decommission some bookies, which configured placement policy, it may lead to writing new entries failing due to not meeting placement policy.



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;

Review Comment:
   Use `List<Long>` instead of `String`



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;
+
+        @Parameter(names = { "-r", "--readOnly" }, description = "Whether to set the bookie nodes of the migrated data"
+                + " to readOnly, the default is true")
+        private boolean readOnly;
+
+        @Parameter(names = {"-p", "--httpPort"}, description = "http port,default is 8080")
+        private String port;
+    }
+
+    @Override
+    public boolean apply(ServerConfiguration conf, ReplicasMigrationFlags cmdFlags) {
+        try {
+            return switchToReadonly(conf, cmdFlags) && replicasMigration(conf, cmdFlags);
+        } catch (Exception e) {
+            throw new UncheckedExecutionException(e.getMessage(), e);
+        }
+    }
+
+    public Set<BookieId> parseBookieIds(String bookieIdsOfMigratedReplica) {
+        Set<BookieId> bookieIdsSet = new HashSet<>();
+        String[] bookieIds = bookieIdsOfMigratedReplica.split(seperator);
+        for (String bookieId : bookieIds) {
+            bookieIdsSet.add(BookieId.parse(bookieId));
+        }
+        return bookieIdsSet;
+    }
+
+    public Set<Long> parseLedgerIds(String ledgerIdsOfMigratedReplica) {
+        Set<Long> ledgerIdSet = new HashSet<>();
+        String[] ledgerIds = ledgerIdsOfMigratedReplica.split(seperator);
+        for (String ledgerId : ledgerIds) {
+            ledgerIdSet.add(Long.parseLong(ledgerId));
+        }
+        return ledgerIdSet;
+    }
+
+    private boolean switchToReadonly(ServerConfiguration conf, ReplicasMigrationFlags flags) throws IOException {
+        if (flags.readOnly && !conf.isHttpServerEnabled()) {
+            throw new RuntimeException("Please enable http service first, config httpServerEnabled is false!");
+        }
+
+        Set<BookieId> bookieIdSet = parseBookieIds(flags.bookieIdsOfMigratedReplica);
+        for (BookieId bookieId : bookieIdSet) {
+            String bookieAddress = bookieId.getId().split(":")[0];
+            String httpAddress = bookieAddress + ":" + flags.port;
+            String url = String.format("http://%s/%s", httpAddress, HttpRouter.BOOKIE_STATE_READONLY);
+            String readOnly = flags.readOnly ? "true" : "false";
+            CloseableHttpResponse response = HttpClientUtils.doPutRequest(url, readOnly, "UTF-8");
+            int responseCode = response.getStatusLine().getStatusCode();
+            if (responseCode != HttpServer.StatusCode.OK.getValue()) {
+                LOG.error(String.format("Set readonly to %s failed! bookieId:%s, responseCode:%s ",
+                        readOnly, bookieId, responseCode));
+                return false;
+            }
+            LOG.info(String.format("Set readonly to %s success! bookieId:%s, responseCode:%s",
+                    readOnly, bookieId, responseCode));
+        }
+        return true;
+    }
+
+    private boolean replicasMigration(ServerConfiguration conf, ReplicasMigrationFlags flags)
+            throws BKException, InterruptedException, IOException {
+        ClientConfiguration adminConf = new ClientConfiguration(conf);
+        BookKeeperAdmin admin = new BookKeeperAdmin(adminConf);
+        try {
+            Map<Long, Set<BookieId>> toMigratedLedgerAndBookieMap = new HashMap<>();
+            Set<BookieId> bookieIds = parseBookieIds(flags.bookieIdsOfMigratedReplica);
+            Set<Long> ledgerIdsToMigrate = null;
+            if (!flags.ledgerIdsOfMigratedReplica.equals(ALL)) {
+                ledgerIdsToMigrate = parseLedgerIds(flags.ledgerIdsOfMigratedReplica);
+            }
+            Map<BookieId, Set<Long>> bookieLedgerMaps = admin.listLedgers(bookieIds);
+            for (Map.Entry<BookieId, Set<Long>> bookieIdSetEntry : bookieLedgerMaps.entrySet()) {
+                Set<Long> ledgersOnBookieId = bookieIdSetEntry.getValue();
+                if (!flags.ledgerIdsOfMigratedReplica.equals(ALL)) {
+                    ledgersOnBookieId.retainAll(ledgerIdsToMigrate);

Review Comment:
   `ledgerIdsToMigrate ` maybe null



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;
+
+        @Parameter(names = { "-r", "--readOnly" }, description = "Whether to set the bookie nodes of the migrated data"
+                + " to readOnly, the default is true")
+        private boolean readOnly;
+
+        @Parameter(names = {"-p", "--httpPort"}, description = "http port,default is 8080")
+        private String port;
+    }
+
+    @Override
+    public boolean apply(ServerConfiguration conf, ReplicasMigrationFlags cmdFlags) {
+        try {
+            return switchToReadonly(conf, cmdFlags) && replicasMigration(conf, cmdFlags);
+        } catch (Exception e) {
+            throw new UncheckedExecutionException(e.getMessage(), e);
+        }
+    }
+
+    public Set<BookieId> parseBookieIds(String bookieIdsOfMigratedReplica) {
+        Set<BookieId> bookieIdsSet = new HashSet<>();
+        String[] bookieIds = bookieIdsOfMigratedReplica.split(seperator);
+        for (String bookieId : bookieIds) {
+            bookieIdsSet.add(BookieId.parse(bookieId));
+        }
+        return bookieIdsSet;
+    }
+
+    public Set<Long> parseLedgerIds(String ledgerIdsOfMigratedReplica) {
+        Set<Long> ledgerIdSet = new HashSet<>();
+        String[] ledgerIds = ledgerIdsOfMigratedReplica.split(seperator);
+        for (String ledgerId : ledgerIds) {
+            ledgerIdSet.add(Long.parseLong(ledgerId));
+        }
+        return ledgerIdSet;
+    }
+
+    private boolean switchToReadonly(ServerConfiguration conf, ReplicasMigrationFlags flags) throws IOException {
+        if (flags.readOnly && !conf.isHttpServerEnabled()) {
+            throw new RuntimeException("Please enable http service first, config httpServerEnabled is false!");
+        }
+
+        Set<BookieId> bookieIdSet = parseBookieIds(flags.bookieIdsOfMigratedReplica);
+        for (BookieId bookieId : bookieIdSet) {
+            String bookieAddress = bookieId.getId().split(":")[0];
+            String httpAddress = bookieAddress + ":" + flags.port;
+            String url = String.format("http://%s/%s", httpAddress, HttpRouter.BOOKIE_STATE_READONLY);
+            String readOnly = flags.readOnly ? "true" : "false";
+            CloseableHttpResponse response = HttpClientUtils.doPutRequest(url, readOnly, "UTF-8");
+            int responseCode = response.getStatusLine().getStatusCode();
+            if (responseCode != HttpServer.StatusCode.OK.getValue()) {
+                LOG.error(String.format("Set readonly to %s failed! bookieId:%s, responseCode:%s ",
+                        readOnly, bookieId, responseCode));
+                return false;

Review Comment:
   If we want to decommission 3 bookies, <bookie0, bookie1, bookie2>, and <bookie0, bookie1> set to readOnly mode succeed, and bookie2 set to readOnly failed, do we need to rollback <bookie0, bookie1> to the previous mode?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;
+
+        @Parameter(names = { "-r", "--readOnly" }, description = "Whether to set the bookie nodes of the migrated data"
+                + " to readOnly, the default is true")
+        private boolean readOnly;

Review Comment:
   Please configure the default value to false.



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;
+
+        @Parameter(names = { "-r", "--readOnly" }, description = "Whether to set the bookie nodes of the migrated data"
+                + " to readOnly, the default is true")
+        private boolean readOnly;
+
+        @Parameter(names = {"-p", "--httpPort"}, description = "http port,default is 8080")
+        private String port;

Review Comment:
   The type is `int`?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;
+
+        @Parameter(names = { "-r", "--readOnly" }, description = "Whether to set the bookie nodes of the migrated data"
+                + " to readOnly, the default is true")
+        private boolean readOnly;
+
+        @Parameter(names = {"-p", "--httpPort"}, description = "http port,default is 8080")
+        private String port;
+    }
+
+    @Override
+    public boolean apply(ServerConfiguration conf, ReplicasMigrationFlags cmdFlags) {
+        try {
+            return switchToReadonly(conf, cmdFlags) && replicasMigration(conf, cmdFlags);
+        } catch (Exception e) {
+            throw new UncheckedExecutionException(e.getMessage(), e);
+        }
+    }
+
+    public Set<BookieId> parseBookieIds(String bookieIdsOfMigratedReplica) {
+        Set<BookieId> bookieIdsSet = new HashSet<>();
+        String[] bookieIds = bookieIdsOfMigratedReplica.split(seperator);
+        for (String bookieId : bookieIds) {
+            bookieIdsSet.add(BookieId.parse(bookieId));
+        }
+        return bookieIdsSet;
+    }
+
+    public Set<Long> parseLedgerIds(String ledgerIdsOfMigratedReplica) {
+        Set<Long> ledgerIdSet = new HashSet<>();
+        String[] ledgerIds = ledgerIdsOfMigratedReplica.split(seperator);
+        for (String ledgerId : ledgerIds) {
+            ledgerIdSet.add(Long.parseLong(ledgerId));
+        }
+        return ledgerIdSet;
+    }
+
+    private boolean switchToReadonly(ServerConfiguration conf, ReplicasMigrationFlags flags) throws IOException {
+        if (flags.readOnly && !conf.isHttpServerEnabled()) {
+            throw new RuntimeException("Please enable http service first, config httpServerEnabled is false!");
+        }
+
+        Set<BookieId> bookieIdSet = parseBookieIds(flags.bookieIdsOfMigratedReplica);
+        for (BookieId bookieId : bookieIdSet) {
+            String bookieAddress = bookieId.getId().split(":")[0];
+            String httpAddress = bookieAddress + ":" + flags.port;
+            String url = String.format("http://%s/%s", httpAddress, HttpRouter.BOOKIE_STATE_READONLY);
+            String readOnly = flags.readOnly ? "true" : "false";
+            CloseableHttpResponse response = HttpClientUtils.doPutRequest(url, readOnly, "UTF-8");
+            int responseCode = response.getStatusLine().getStatusCode();
+            if (responseCode != HttpServer.StatusCode.OK.getValue()) {
+                LOG.error(String.format("Set readonly to %s failed! bookieId:%s, responseCode:%s ",
+                        readOnly, bookieId, responseCode));
+                return false;
+            }
+            LOG.info(String.format("Set readonly to %s success! bookieId:%s, responseCode:%s",
+                    readOnly, bookieId, responseCode));
+        }
+        return true;
+    }
+
+    private boolean replicasMigration(ServerConfiguration conf, ReplicasMigrationFlags flags)
+            throws BKException, InterruptedException, IOException {
+        ClientConfiguration adminConf = new ClientConfiguration(conf);
+        BookKeeperAdmin admin = new BookKeeperAdmin(adminConf);
+        try {

Review Comment:
   Suggest `try (BookKeeperAdmin admin = new BookKeeperAdmin(adminConf))`



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java:
##########
@@ -1951,6 +1953,56 @@ public int runCmd(CommandLine cmdLine) throws Exception {
         }
     }
 
+    /**
+     * Migrate the specified ledger data on some bookie to other bookies.
+     * */
+    class ReplicasMigrationCmd extends MyCommand {
+
+        ReplicasMigrationCmd() {
+            super(CMD_REPLICASMIGRATION);
+            opts.addOption("b", "bookieIds", true,
+                    "Bookies corresponding to the migrated replica");
+            opts.addOption("l", "ledgerIds", true,
+                    "The ledgerIds corresponding to the migrated replica");
+            opts.addOption("r", "readOnly", true,
+                    "Whether to set the bookie nodes of the migrated data to readOnly, the default is true");
+            opts.addOption("p", "httpPort", true, "http port,default is 8080");

Review Comment:
   required = false?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookies/ReplicasMigrationCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.bookies;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpRouter;
+import org.apache.bookkeeper.http.HttpServer;
+import org.apache.bookkeeper.meta.MetadataDrivers;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommand;
+import org.apache.bookkeeper.tools.framework.CliFlags;
+import org.apache.bookkeeper.tools.framework.CliSpec;
+import org.apache.bookkeeper.util.HttpClientUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Migrate the data of the specified replica to other bookie nodes.
+ * */
+public class ReplicasMigrationCommand extends BookieCommand<ReplicasMigrationCommand.ReplicasMigrationFlags> {
+    static final Logger LOG = LoggerFactory.getLogger(ReplicasMigrationCommand.class);
+    private static final String NAME = "replicasMigration";
+    private static final String DESC = "Migrate the data of the specified replica to other bookie nodes";
+    private static final String ALL = "ALL";
+    private final String seperator = ",";
+    public ReplicasMigrationCommand() {
+        this(new ReplicasMigrationFlags());
+    }
+    private ReplicasMigrationCommand(ReplicasMigrationFlags flags) {
+        super(CliSpec.<ReplicasMigrationFlags>newBuilder()
+                .withName(NAME)
+                .withDescription(DESC)
+                .withFlags(flags)
+                .build());
+    }
+
+    /**
+     * Flags for replicasMigration command.
+     */
+    @Accessors(fluent = true)
+    @Setter
+    public static class ReplicasMigrationFlags extends CliFlags {
+
+        @Parameter(names = { "-b", "--bookieIds" }, description = "Bookies corresponding to the migrated replica, "
+                + "eg: bookieId1,bookieId2,bookieId3")
+        private String bookieIdsOfMigratedReplica;
+
+        @Parameter(names = { "-l", "--ledgerIds" }, description = "The ledgerIds corresponding to the migrated replica,"
+                + "eg: ledgerId1,ledgerId2,ledgerId3. The default is ALL, which means that all replica data on these "
+                + "bookie nodes will be migrated to other bookie nodes")
+        private String ledgerIdsOfMigratedReplica;
+
+        @Parameter(names = { "-r", "--readOnly" }, description = "Whether to set the bookie nodes of the migrated data"
+                + " to readOnly, the default is true")
+        private boolean readOnly;
+
+        @Parameter(names = {"-p", "--httpPort"}, description = "http port,default is 8080")
+        private String port;
+    }
+
+    @Override
+    public boolean apply(ServerConfiguration conf, ReplicasMigrationFlags cmdFlags) {
+        try {
+            return switchToReadonly(conf, cmdFlags) && replicasMigration(conf, cmdFlags);
+        } catch (Exception e) {
+            throw new UncheckedExecutionException(e.getMessage(), e);
+        }
+    }
+
+    public Set<BookieId> parseBookieIds(String bookieIdsOfMigratedReplica) {
+        Set<BookieId> bookieIdsSet = new HashSet<>();
+        String[] bookieIds = bookieIdsOfMigratedReplica.split(seperator);
+        for (String bookieId : bookieIds) {
+            bookieIdsSet.add(BookieId.parse(bookieId));
+        }
+        return bookieIdsSet;
+    }
+
+    public Set<Long> parseLedgerIds(String ledgerIdsOfMigratedReplica) {
+        Set<Long> ledgerIdSet = new HashSet<>();
+        String[] ledgerIds = ledgerIdsOfMigratedReplica.split(seperator);
+        for (String ledgerId : ledgerIds) {
+            ledgerIdSet.add(Long.parseLong(ledgerId));
+        }
+        return ledgerIdSet;
+    }
+
+    private boolean switchToReadonly(ServerConfiguration conf, ReplicasMigrationFlags flags) {
+        if (flags.readOnly && !conf.isHttpServerEnabled()) {
+            throw new RuntimeException("Please enable http service first, config httpServerEnabled is false!");
+        }
+
+        Set<BookieId> bookieIdSet = parseBookieIds(flags.bookieIdsOfMigratedReplica);
+        for (BookieId bookieId : bookieIdSet) {
+            String bookieAddress = bookieId.getId().split(":")[0];

Review Comment:
   +1, we need to use `resolve` the ID



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org