You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/08/20 04:08:09 UTC
[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #2768: Add metrics and internal command for AutoRecovery
hangc0276 commented on a change in pull request #2768:
URL: https://github.com/apache/bookkeeper/pull/2768#discussion_r692641770
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/autorecovery/QueryAutoRecoveryStatusCommand.java
##########
@@ -0,0 +1,144 @@
+package org.apache.bookkeeper.tools.cli.commands.autorecovery;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import com.google.protobuf.TextFormat;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.LedgerManager;
+import org.apache.bookkeeper.meta.UnderreplicatedLedger;
+import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
+import org.apache.bookkeeper.meta.zk.ZKMetadataDriverBase;
+import org.apache.bookkeeper.proto.DataFormats;
+import org.apache.bookkeeper.replication.ReplicationException;
+import org.apache.bookkeeper.tools.cli.commands.bookie.ListLedgersCommand;
+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.BookKeeperConstants;
+import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooKeeper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URI;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithLedgerManagerFactory;
+import static org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager.getBasePath;
+import static org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager.getUrLedgerLockZnode;
+
+public class QueryAutoRecoveryStatusCommand extends BookieCommand<QueryAutoRecoveryStatusCommand.QFlags> {
+ static final Logger LOG = LoggerFactory.getLogger(QueryAutoRecoveryStatusCommand.class);
+ private static final String NAME = "queryautorecoverystatus";
+ private static final String DESC = "Query autorecovery status.";
+
+ public QueryAutoRecoveryStatusCommand() {
+ super(CliSpec.<QFlags>newBuilder()
+ .withName(NAME)
+ .withDescription(DESC)
+ .withFlags(new QFlags())
+ .build());
+ }
+
+ @Override
+ public boolean apply(ServerConfiguration conf, QFlags cmdFlags) {
+ try {
+ return handler(conf, cmdFlags);
+ } catch (Exception e) {
+ throw new UncheckedExecutionException(e.getMessage(), e);
+ }
+ }
+
+ /**
+ * Flags for list under replicated command.
+ */
+ @Accessors(fluent = true)
+ @Setter
+ public static class QFlags extends CliFlags{
+ @Parameter(names = {"-v", "--verbose"}, description = "list recovering detailed ledger info")
+ private Boolean verbose = false;
+ }
+
+
+ /*
+ Print Message format is like this:
+
+ CurrentRecoverLedgerInfo:
+ LedgerId: BookieId: LedgerSize:(detail)
+ LedgerId: BookieId: LedgerSize:(detail)
+ */
+ public boolean handler(ServerConfiguration conf, QFlags flag) throws Exception {
+ ZooKeeper zk = null;
+ try {
+ String metadataServiceUri = conf.getMetadataServiceUri();
+ String zkServers = ZKMetadataDriverBase.getZKServersFromServiceUri(URI.create(metadataServiceUri));
+ zk = ZooKeeperClient.newBuilder()
+ .connectString(zkServers)
+ .sessionTimeoutMs(conf.getZkTimeout())
+ .build();
+
+ String basePath = getBasePath(ZKMetadataDriverBase.resolveZkLedgersRootPath(conf));
+ String urLockPath = basePath + '/' + BookKeeperConstants.UNDER_REPLICATION_LOCK;
+
+ List<String> locks = zk.getChildren(urLockPath, false);
+
+ LOG.info("CurrentRecoverLedgerInfo:");
+ if (locks.size() > 0) {
+ for (int i = 0;i < locks.size();i ++) {
+ String replicationWorkerId = null;
+ String ledgerIdStr = locks.get(i);
+ try {
+ byte[] lockData = zk.getData(getUrLedgerLockZnode(urLockPath, Long.parseLong(ledgerIdStr)), false, null);
+ DataFormats.LockDataFormat.Builder lockDataBuilder = DataFormats.LockDataFormat.newBuilder();
+ TextFormat.merge(new String(lockData, UTF_8), lockDataBuilder);
+ DataFormats.LockDataFormat lock = lockDataBuilder.build();
+ replicationWorkerId = lock.getBookieId();
+ long ledgerId = Long.parseLong(ledgerIdStr);
+ if (!flag.verbose) {
+ LOG.info("\tLedgerId:"+ledgerIdStr +"\tBookieId:"+replicationWorkerId);
Review comment:
Please format the `LOG.info`, do not use string concat.
##########
File path: tools/ledger/src/test/java/org/apache/bookkeeper/tools/cli/commands/autorecovery/QueryAutoRecoveryStatusCommandTest.java
##########
@@ -0,0 +1,123 @@
+package org.apache.bookkeeper.tools.cli.commands.autorecovery;
+
+import com.google.common.collect.Lists;
+import org.apache.bookkeeper.client.BookKeeper;
+import org.apache.bookkeeper.client.LedgerMetadataBuilder;
+import org.apache.bookkeeper.client.api.LedgerMetadata;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.*;
Review comment:
do not use `import *`
##########
File path: tools/ledger/src/test/java/org/apache/bookkeeper/tools/cli/commands/autorecovery/QueryAutoRecoveryStatusCommandTest.java
##########
@@ -0,0 +1,123 @@
+package org.apache.bookkeeper.tools.cli.commands.autorecovery;
+
+import com.google.common.collect.Lists;
+import org.apache.bookkeeper.client.BookKeeper;
+import org.apache.bookkeeper.client.LedgerMetadataBuilder;
+import org.apache.bookkeeper.client.api.LedgerMetadata;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.*;
+import org.apache.bookkeeper.meta.zk.ZKMetadataDriverBase;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.net.BookieSocketAddress;
+import org.apache.bookkeeper.proto.BookieAddressResolver;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommandTestBase;
+import org.apache.bookkeeper.tools.cli.helpers.CommandHelpers;
+import org.apache.bookkeeper.versioning.LongVersion;
+import org.apache.bookkeeper.versioning.Versioned;
+import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.net.URI;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+
+import static org.mockito.ArgumentMatchers.*;
Review comment:
same with above
##########
File path: tools/ledger/src/test/java/org/apache/bookkeeper/tools/cli/commands/autorecovery/QueryAutoRecoveryStatusCommandTest.java
##########
@@ -0,0 +1,123 @@
+package org.apache.bookkeeper.tools.cli.commands.autorecovery;
+
+import com.google.common.collect.Lists;
+import org.apache.bookkeeper.client.BookKeeper;
+import org.apache.bookkeeper.client.LedgerMetadataBuilder;
+import org.apache.bookkeeper.client.api.LedgerMetadata;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.*;
+import org.apache.bookkeeper.meta.zk.ZKMetadataDriverBase;
+import org.apache.bookkeeper.net.BookieId;
+import org.apache.bookkeeper.net.BookieSocketAddress;
+import org.apache.bookkeeper.proto.BookieAddressResolver;
+import org.apache.bookkeeper.tools.cli.helpers.BookieCommandTestBase;
+import org.apache.bookkeeper.tools.cli.helpers.CommandHelpers;
+import org.apache.bookkeeper.versioning.LongVersion;
+import org.apache.bookkeeper.versioning.Versioned;
+import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.net.URI;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+
+import static org.mockito.ArgumentMatchers.*;
+import static org.powermock.api.mockito.PowerMockito.mock;
+import static org.powermock.api.mockito.PowerMockito.when;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ QueryAutoRecoveryStatusCommand.class, ZKMetadataDriverBase.class, ZooKeeperClient.class,
+ CommandHelpers.class, MetadataDrivers.class
+})
+public class QueryAutoRecoveryStatusCommandTest extends BookieCommandTestBase {
+ public QueryAutoRecoveryStatusCommandTest() {
+ super(3, 0);
+ }
+
+ @Override
+ public void setup() throws Exception {
+ super.setup();
+
+ PowerMockito.whenNew(ServerConfiguration.class).withNoArguments().thenReturn(conf);
+
+ PowerMockito.mockStatic(ZKMetadataDriverBase.class);
+ PowerMockito.when(ZKMetadataDriverBase.getZKServersFromServiceUri(eq(URI.create(conf.getMetadataServiceUri()))))
+ .thenReturn("");
+
+ ZooKeeperClient zk = mock(ZooKeeperClient.class);
+ ZooKeeperClient.Builder builder = mock(ZooKeeperClient.Builder.class);
+ PowerMockito.mockStatic(ZooKeeperClient.class);
+ PowerMockito.when(ZooKeeperClient.newBuilder()).thenReturn(builder);
+ when(builder.connectString(anyString())).thenReturn(builder);
+ when(builder.sessionTimeoutMs(anyInt())).thenReturn(builder);
+ when(builder.build()).thenReturn(zk);
+
+ BookieId bookieId = BookieId.parse(UUID.randomUUID().toString());
+
+ LinkedList<String> list = new LinkedList<>();
+ list.push("1");
+
+ when(zk.getChildren(anyString(), anyBoolean())).thenReturn(list);
+ when(zk.getData(any(), anyBoolean(), any())).thenReturn(ZkLedgerUnderreplicationManager.getLockData());
+
+
+ LedgerManagerFactory ledgerManagerFactory = mock(LedgerManagerFactory.class);
+
+ PowerMockito.mockStatic(MetadataDrivers.class);
+ PowerMockito.doAnswer(invocationOnMock -> {
+ Function<LedgerManagerFactory, ?> function = invocationOnMock.getArgument(1);
+ function.apply(ledgerManagerFactory);
+ return true;
+ }).when(MetadataDrivers.class, "runFunctionWithLedgerManagerFactory", any(ServerConfiguration.class),
+ any(Function.class));
+
+ LedgerManager ledgerManager = mock(LedgerManager.class);
+
+ when(ledgerManagerFactory.newLedgerManager()).thenReturn(ledgerManager);
+
+ List<BookieId> ensemble = Lists.newArrayList(new BookieSocketAddress("192.0.2.1", 1234).toBookieId(),
+ new BookieSocketAddress("192.0.2.2", 1234).toBookieId(),
+ new BookieSocketAddress("192.0.2.3", 1234).toBookieId());
+ LedgerMetadata metadata = LedgerMetadataBuilder.create()
+ .withId(11112233)
+ .withClosedState()
+ .withLength(1000000)
+ .withLastEntryId(2000011)
+ .withEnsembleSize(3).withWriteQuorumSize(2).withAckQuorumSize(2)
+ .withPassword("passwd".getBytes())
+ .withDigestType(BookKeeper.DigestType.CRC32.toApiDigestType())
+ .newEnsembleEntry(0L, ensemble).build();
+
+ CompletableFuture<Versioned<LedgerMetadata>> promise = new CompletableFuture<>();
+ Versioned<LedgerMetadata> vmeta = new Versioned<LedgerMetadata>(metadata, new LongVersion(1000));
+ promise.complete(vmeta);
+
+ when(ledgerManager.readLedgerMetadata(1)).thenReturn(promise);
+
+
+ PowerMockito.mockStatic(CommandHelpers.class);
+ PowerMockito.when(CommandHelpers
+ .getBookieSocketAddrStringRepresentation(
+ eq(bookieId), any(BookieAddressResolver.class))).thenReturn("");
+ }
+
+ @Test
Review comment:
add timeout for test
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerFragmentReplicator.java
##########
@@ -310,6 +327,8 @@ private void recoverLedgerFragmentEntry(final Long entryId,
final long ledgerId = lh.getId();
final AtomicInteger numCompleted = new AtomicInteger(0);
final AtomicBoolean completed = new AtomicBoolean(false);
+ Stopwatch readEntryWatcher = Stopwatch.createUnstarted();
Review comment:
You can just just start to record the start timestamp instead of Stopwatch
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/autorecovery/QueryAutoRecoveryStatusCommand.java
##########
@@ -0,0 +1,144 @@
+package org.apache.bookkeeper.tools.cli.commands.autorecovery;
+
+import com.beust.jcommander.Parameter;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import com.google.protobuf.TextFormat;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.BKException;
+import org.apache.bookkeeper.client.BookKeeperAdmin;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.LedgerManager;
+import org.apache.bookkeeper.meta.UnderreplicatedLedger;
+import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
+import org.apache.bookkeeper.meta.zk.ZKMetadataDriverBase;
+import org.apache.bookkeeper.proto.DataFormats;
+import org.apache.bookkeeper.replication.ReplicationException;
+import org.apache.bookkeeper.tools.cli.commands.bookie.ListLedgersCommand;
+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.BookKeeperConstants;
+import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooKeeper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URI;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithLedgerManagerFactory;
+import static org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager.getBasePath;
+import static org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager.getUrLedgerLockZnode;
+
+public class QueryAutoRecoveryStatusCommand extends BookieCommand<QueryAutoRecoveryStatusCommand.QFlags> {
+ static final Logger LOG = LoggerFactory.getLogger(QueryAutoRecoveryStatusCommand.class);
+ private static final String NAME = "queryautorecoverystatus";
+ private static final String DESC = "Query autorecovery status.";
+
+ public QueryAutoRecoveryStatusCommand() {
+ super(CliSpec.<QFlags>newBuilder()
+ .withName(NAME)
+ .withDescription(DESC)
+ .withFlags(new QFlags())
+ .build());
+ }
+
+ @Override
+ public boolean apply(ServerConfiguration conf, QFlags cmdFlags) {
+ try {
+ return handler(conf, cmdFlags);
+ } catch (Exception e) {
+ throw new UncheckedExecutionException(e.getMessage(), e);
+ }
+ }
+
+ /**
+ * Flags for list under replicated command.
+ */
+ @Accessors(fluent = true)
+ @Setter
+ public static class QFlags extends CliFlags{
+ @Parameter(names = {"-v", "--verbose"}, description = "list recovering detailed ledger info")
+ private Boolean verbose = false;
+ }
+
+
+ /*
+ Print Message format is like this:
+
+ CurrentRecoverLedgerInfo:
+ LedgerId: BookieId: LedgerSize:(detail)
+ LedgerId: BookieId: LedgerSize:(detail)
+ */
+ public boolean handler(ServerConfiguration conf, QFlags flag) throws Exception {
+ ZooKeeper zk = null;
+ try {
+ String metadataServiceUri = conf.getMetadataServiceUri();
+ String zkServers = ZKMetadataDriverBase.getZKServersFromServiceUri(URI.create(metadataServiceUri));
+ zk = ZooKeeperClient.newBuilder()
+ .connectString(zkServers)
+ .sessionTimeoutMs(conf.getZkTimeout())
+ .build();
+
+ String basePath = getBasePath(ZKMetadataDriverBase.resolveZkLedgersRootPath(conf));
+ String urLockPath = basePath + '/' + BookKeeperConstants.UNDER_REPLICATION_LOCK;
+
+ List<String> locks = zk.getChildren(urLockPath, false);
+
+ LOG.info("CurrentRecoverLedgerInfo:");
+ if (locks.size() > 0) {
+ for (int i = 0;i < locks.size();i ++) {
Review comment:
Please format the code of `for (int i = 0;i < locks.size();i ++)`
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org