From c26b006a4546fc4463baaf219354aef635cebea9 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Sun, 5 Sep 2021 10:35:10 +0800 Subject: [PATCH 01/19] step 1 support ZookeeperClient and CoordinatorEventManager --- kafka-impl/pom.xml | 6 + .../handlers/kop/KafkaChannelInitializer.java | 9 +- .../handlers/kop/KafkaProtocolHandler.java | 47 +- .../handlers/kop/KafkaRequestHandler.java | 4 + .../group/CoordinatorEventManager.java | 132 +++++ .../coordinator/group/GroupCoordinator.java | 82 ++- .../group/GroupMetadataManager.java | 29 ++ .../handlers/kop/utils/KopZkClient.java | 122 +++++ .../handlers/kop/utils/ZooKeeperClient.java | 488 ++++++++++++++++++ 9 files changed, 899 insertions(+), 20 deletions(-) create mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java create mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java create mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java diff --git a/kafka-impl/pom.xml b/kafka-impl/pom.xml index 38531f63a0..b955955481 100644 --- a/kafka-impl/pom.xml +++ b/kafka-impl/pom.xml @@ -33,6 +33,12 @@ + + org.apache.kafka + kafka_2.11 + 2.0.0-cp1 + compile + diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java index ab86d9ea2c..5253db2288 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java @@ -20,6 +20,7 @@ import io.netty.handler.codec.LengthFieldBasedFrameDecoder; import io.netty.handler.codec.LengthFieldPrepender; import io.netty.handler.ssl.SslHandler; +import io.streamnative.pulsar.handlers.kop.coordinator.group.CoordinatorEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.transaction.TransactionCoordinator; import io.streamnative.pulsar.handlers.kop.stats.StatsLogger; @@ -44,6 +45,8 @@ public class KafkaChannelInitializer extends ChannelInitializer { @Getter private final GroupCoordinator groupCoordinator; @Getter + private final CoordinatorEventManager coordinatorEventManager; + @Getter private final TransactionCoordinator transactionCoordinator; private final AdminManager adminManager; @Getter @@ -59,6 +62,7 @@ public class KafkaChannelInitializer extends ChannelInitializer { public KafkaChannelInitializer(PulsarService pulsarService, KafkaServiceConfiguration kafkaConfig, GroupCoordinator groupCoordinator, + CoordinatorEventManager coordinatorEventManager, TransactionCoordinator transactionCoordinator, AdminManager adminManager, boolean enableTLS, @@ -69,6 +73,7 @@ public KafkaChannelInitializer(PulsarService pulsarService, this.pulsarService = pulsarService; this.kafkaConfig = kafkaConfig; this.groupCoordinator = groupCoordinator; + this.coordinatorEventManager = coordinatorEventManager; this.transactionCoordinator = transactionCoordinator; this.adminManager = adminManager; this.enableTls = enableTLS; @@ -93,8 +98,8 @@ protected void initChannel(SocketChannel ch) throws Exception { new LengthFieldBasedFrameDecoder(MAX_FRAME_LENGTH, 0, 4, 0, 4)); ch.pipeline().addLast("handler", new KafkaRequestHandler(pulsarService, kafkaConfig, - groupCoordinator, transactionCoordinator, adminManager, localBrokerDataCache, - enableTls, advertisedEndPoint, statsLogger)); + groupCoordinator, coordinatorEventManager, transactionCoordinator, adminManager, + localBrokerDataCache, enableTls, advertisedEndPoint, statsLogger)); } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java index 2fe8eeffc3..f90cd2c89f 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import io.netty.channel.ChannelInitializer; import io.netty.channel.socket.SocketChannel; +import io.streamnative.pulsar.handlers.kop.coordinator.group.CoordinatorEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupConfig; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.group.OffsetConfig; @@ -30,7 +31,9 @@ import io.streamnative.pulsar.handlers.kop.stats.StatsLogger; import io.streamnative.pulsar.handlers.kop.utils.ConfigurationUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; +import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.MetadataUtils; +import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperUtils; import io.streamnative.pulsar.handlers.kop.utils.timer.SystemTimer; import java.net.InetSocketAddress; @@ -95,6 +98,10 @@ public class KafkaProtocolHandler implements ProtocolHandler { private GroupCoordinator groupCoordinator; @Getter private TransactionCoordinator transactionCoordinator; + @Getter + private CoordinatorEventManager coordinatorEventManager; + @Getter + private KopZkClient kopZkClient; /** * Listener for the changing of topic that stores offsets of consumer group. @@ -320,6 +327,10 @@ public void start(BrokerService service) { throw new IllegalStateException(e); } + kopZkClient = createKopZkClient(kafkaConfig.getZookeeperServers()); + // init coordinatorEventManager + coordinatorEventManager = new CoordinatorEventManager(); + // init and start group coordinator startGroupCoordinator(pulsarClient); // and listener for Offset topics load/unload @@ -375,14 +386,16 @@ public Map> newChannelIniti case PLAINTEXT: case SASL_PLAINTEXT: builder.put(endPoint.getInetAddress(), new KafkaChannelInitializer(brokerService.getPulsar(), - kafkaConfig, groupCoordinator, transactionCoordinator, adminManager, false, - advertisedEndPoint, rootStatsLogger.scope(SERVER_SCOPE), localBrokerDataCache)); + kafkaConfig, groupCoordinator, coordinatorEventManager, + transactionCoordinator, adminManager, false, + advertisedEndPoint, rootStatsLogger.scope(SERVER_SCOPE), localBrokerDataCache)); break; case SSL: case SASL_SSL: builder.put(endPoint.getInetAddress(), new KafkaChannelInitializer(brokerService.getPulsar(), - kafkaConfig, groupCoordinator, transactionCoordinator, adminManager, true, - advertisedEndPoint, rootStatsLogger.scope(SERVER_SCOPE), localBrokerDataCache)); + kafkaConfig, groupCoordinator, coordinatorEventManager, + transactionCoordinator, adminManager, true, + advertisedEndPoint, rootStatsLogger.scope(SERVER_SCOPE), localBrokerDataCache)); break; } }); @@ -426,13 +439,15 @@ public void startGroupCoordinator(PulsarClient pulsarClient) { .build(); this.groupCoordinator = GroupCoordinator.of( - (PulsarClientImpl) pulsarClient, - groupConfig, - offsetConfig, - SystemTimer.builder() - .executorName("group-coordinator-timer") - .build(), - Time.SYSTEM + (PulsarClientImpl) pulsarClient, + groupConfig, + offsetConfig, + SystemTimer.builder() + .executorName("group-coordinator-timer") + .build(), + Time.SYSTEM, + coordinatorEventManager, + kopZkClient ); // always enable metadata expiration this.groupCoordinator.startup(true); @@ -499,4 +514,14 @@ private void loadTxnLogTopics(TransactionCoordinator txnCoordinator) throws Exce public static @NonNull LookupClient getLookupClient(final PulsarService pulsarService) { return LOOKUP_CLIENT_MAP.computeIfAbsent(pulsarService, ignored -> new LookupClient(pulsarService)); } + + private static KopZkClient createKopZkClient(String zkConnect) { + ZooKeeperClient zooKeeperClient = new ZooKeeperClient(zkConnect, + 30000, + 15000, + Integer.MAX_VALUE); + + return new KopZkClient(zooKeeperClient); + } + } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java index 59c5bbed9f..26afb6c8ba 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java @@ -31,6 +31,7 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; +import io.streamnative.pulsar.handlers.kop.coordinator.group.CoordinatorEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupOverview; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupSummary; @@ -191,6 +192,7 @@ public class KafkaRequestHandler extends KafkaCommandDecoder { private final PulsarService pulsarService; private final KafkaTopicManager topicManager; private final GroupCoordinator groupCoordinator; + private final CoordinatorEventManager coordinatorEventManager; private final TransactionCoordinator transactionCoordinator; private final String clusterName; @@ -240,6 +242,7 @@ public class KafkaRequestHandler extends KafkaCommandDecoder { public KafkaRequestHandler(PulsarService pulsarService, KafkaServiceConfiguration kafkaConfig, GroupCoordinator groupCoordinator, + CoordinatorEventManager coordinatorEventManager, TransactionCoordinator transactionCoordinator, AdminManager adminManager, MetadataCache localBrokerDataCache, @@ -249,6 +252,7 @@ public KafkaRequestHandler(PulsarService pulsarService, super(statsLogger, kafkaConfig); this.pulsarService = pulsarService; this.groupCoordinator = groupCoordinator; + this.coordinatorEventManager = coordinatorEventManager; this.transactionCoordinator = transactionCoordinator; this.clusterName = kafkaConfig.getClusterName(); this.executor = pulsarService.getExecutor(); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java new file mode 100644 index 0000000000..c5010507dd --- /dev/null +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java @@ -0,0 +1,132 @@ +package io.streamnative.pulsar.handlers.kop.coordinator.group; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.locks.ReentrantLock; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class CoordinatorEventManager { + private static final String coordinatorEventThreadName = "coordinator-event-thread"; + private final ReentrantLock putLock = new ReentrantLock(); + private static final LinkedBlockingQueue queue = + new LinkedBlockingQueue<>(); + private final CoordinatorEventThread thread = + new CoordinatorEventThread(coordinatorEventThreadName); + + public void start() { + thread.start(); + } + + public void close() { + thread.shutdown(); + } + + + public void put(GroupCoordinator.CoordinatorEvent event) { + try { + putLock.lock(); + queue.put(event); + } catch (InterruptedException e) { + log.error("Error put event {} to coordinator event queue {}", event, e); + } finally { + putLock.unlock(); + } + } + + public void clearAndPut(GroupCoordinator.CoordinatorEvent event) { + try { + putLock.lock(); + queue.clear(); + put(event); + } finally { + putLock.unlock(); + } + } + + static class CoordinatorEventThread extends Thread { + private final String threadName; + private final CountDownLatch shutdownInitiated = new CountDownLatch(1); + private final CountDownLatch shutdownComplete = new CountDownLatch(1); + + public CoordinatorEventThread(String name) { + this.threadName = name; + } + + private void doWork() { + GroupCoordinator.CoordinatorEvent event = null; + try { + event = queue.take(); + event.process(); + } catch (InterruptedException e) { + log.error("Error processing event {}, {}", event, e); + } + + } + + @Override + public void run() { + log.info("Starting"); + try { + while (isRunning()) { + doWork(); + } + } catch (Exception e) { + shutdownInitiated.countDown(); + shutdownComplete.countDown(); + log.info("Stopped"); + super.stop(); + return; + } finally { + shutdownComplete.countDown(); + } + log.info("Stopped"); + } + + @Override + public synchronized void start() { + setName(); + super.start(); + } + + private void setName() { + super.setName(threadName); + } + + public void shutdown() { + try { + initiateShutdown(); + awaitShutdown(); + } catch (InterruptedException e) { + log.error("shut down {} exception {}", threadName, e); + } + + } + + public boolean isShutdownComplete() { + return shutdownComplete.getCount() == 0; + } + + public boolean initiateShutdown() { + synchronized (this) { + if (isRunning()) { + log.info("Shutting down"); + shutdownInitiated.countDown(); + return true; + } + return false; + } + } + + public void awaitShutdown() throws InterruptedException { + shutdownComplete.await(); + log.info("Shutdown completed"); + } + + private boolean isRunning() { + return shutdownInitiated.getCount() != 0; + } + } + +} diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java index 0dc6627266..206233e0ba 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java @@ -29,6 +29,8 @@ import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupSummary; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; +import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; +import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.GroupKey; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.MemberKey; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationPurgatory; @@ -50,6 +52,9 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; + +import kafka.zookeeper.ZNodeChangeHandler; +import kafka.zookeeper.ZNodeChildChangeHandler; import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.common.util.OrderedScheduler; import org.apache.kafka.common.TopicPartition; @@ -70,6 +75,7 @@ import org.apache.pulsar.client.impl.ReaderBuilderImpl; import org.apache.pulsar.common.schema.KeyValue; import org.apache.pulsar.common.util.FutureUtil; +import org.apache.zookeeper.KeeperException; /** * Group coordinator. @@ -82,7 +88,9 @@ public static GroupCoordinator of( GroupConfig groupConfig, OffsetConfig offsetConfig, Timer timer, - Time time + Time time, + CoordinatorEventManager coordinatorEventManager, + KopZkClient kopZkClient ) { ScheduledExecutorService coordinatorExecutor = OrderedScheduler.newSchedulerBuilder() .name("group-coordinator-executor") @@ -115,11 +123,13 @@ public static GroupCoordinator of( .build(); return new GroupCoordinator( - groupConfig, - metadataManager, - heartbeatPurgatory, - joinPurgatory, - time + groupConfig, + metadataManager, + heartbeatPurgatory, + joinPurgatory, + time, + coordinatorEventManager, + kopZkClient ); } @@ -148,18 +158,26 @@ public static GroupCoordinator of( private final DelayedOperationPurgatory heartbeatPurgatory; private final DelayedOperationPurgatory joinPurgatory; private final Time time; + private final CoordinatorEventManager coordinatorEventManager; + private final KopZkClient kopZkClient; + private DeletionTopicsHandler deletionTopicsHandler; public GroupCoordinator( GroupConfig groupConfig, GroupMetadataManager groupManager, DelayedOperationPurgatory heartbeatPurgatory, DelayedOperationPurgatory joinPurgatory, - Time time) { + Time time, + CoordinatorEventManager coordinatorEventManager, + KopZkClient kopZkClient) { this.groupConfig = groupConfig; this.groupManager = groupManager; this.heartbeatPurgatory = heartbeatPurgatory; this.joinPurgatory = joinPurgatory; this.time = time; + this.coordinatorEventManager = coordinatorEventManager; + this.kopZkClient = kopZkClient; + this.deletionTopicsHandler = new DeletionTopicsHandler(coordinatorEventManager); } /** @@ -168,6 +186,8 @@ public GroupCoordinator( public void startup(boolean enableMetadataExpiration) { log.info("Starting up group coordinator."); groupManager.startup(enableMetadataExpiration); + coordinatorEventManager.start(); + kopZkClient.registerZNodeChildChangeHandler(deletionTopicsHandler); isActive.set(true); log.info("Group coordinator started."); } @@ -180,6 +200,7 @@ public void shutdown() { log.info("Shutting down group coordinator ..."); isActive.set(false); groupManager.shutdown(); + coordinatorEventManager.close(); heartbeatPurgatory.shutdown(); joinPurgatory.shutdown(); log.info("Shutdown group coordinator completely."); @@ -1297,4 +1318,51 @@ private boolean isCoordinatorLoadInProgress(String groupId) { return groupManager.isGroupLoading(groupId); } + class DeletionTopicsHandler implements ZNodeChildChangeHandler { + private final CoordinatorEventManager coordinatorEventManager; + + public DeletionTopicsHandler(CoordinatorEventManager coordinatorEventManager) { + this.coordinatorEventManager = coordinatorEventManager; + } + + @Override + public String path() { + return KopZkClient.getDeleteTopicsZNodePath(); + } + + @Override + public void handleChildChange() { + coordinatorEventManager.put(new DeleteTopicsEvent()); + } + } + + interface CoordinatorEvent { + void process(); + } + + class DeleteTopicsEvent implements CoordinatorEvent { + + @Override + public void process() { +// groupManager + + if (!isActive.get()) { + return; + } + + List topicDeletions = null; + try { + topicDeletions = kopZkClient.getTopicDeletions(); + log.debug("Delete topics listener fired for topics {} to be deleted", topicDeletions); + Iterable groupMetadataIterable = groupManager.currentGroups(); + ZooKeeperClient zkClient = kopZkClient.getZooKeeperClient(); + + } catch (InterruptedException e) { + e.printStackTrace(); + } catch (KeeperException e) { + e.printStackTrace(); + } + } + } + } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java index 0bdd83cc43..159eac702f 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java @@ -31,6 +31,7 @@ import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.CommitRecordMetadataAndOffset; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; +import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.MessageIdUtils; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -55,6 +56,9 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; + +import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; +import kafka.zookeeper.ZNodeChangeHandler; import lombok.Data; import lombok.Getter; import lombok.experimental.Accessors; @@ -1403,4 +1407,29 @@ CompletableFuture> getOffsetsTopicReader(int partitionId) { .createAsync(); }); } + + public static class TopicChangeHandler implements ZNodeChangeHandler { + + @Override + public String path() { + return "/deletetopics"; + } + + @Override + public void handleCreation() { + ZNodeChangeHandler.super.handleCreation(); + } + + @Override + public void handleDeletion() { + ZNodeChangeHandler.super.handleDeletion(); + } + + @Override + public void handleDataChange() { + ZNodeChangeHandler.super.handleDataChange(); + } + } + + } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java new file mode 100644 index 0000000000..676368ee87 --- /dev/null +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java @@ -0,0 +1,122 @@ +package io.streamnative.pulsar.handlers.kop.utils; + +import com.google.api.client.util.Lists; +import com.google.common.collect.Sets; +import com.google.common.collect.Streams; +import kafka.zookeeper.ZNodeChangeHandler; +import kafka.zookeeper.ZNodeChildChangeHandler; +import org.apache.zookeeper.KeeperException; +import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncRequest; +import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncResponse; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +public class KopZkClient { + + private final ZooKeeperClient zooKeeperClient; + + public KopZkClient(ZooKeeperClient zooKeeperClient) { + this.zooKeeperClient = zooKeeperClient; + } + + public ZooKeeperClient getZooKeeperClient() { + return zooKeeperClient; + } + + public void registerZNodeChildChangeHandler(ZNodeChildChangeHandler zNodeChildChangeHandler) { + zooKeeperClient.registerZNodeChildChangeHandler(zNodeChildChangeHandler); + } + + public void unregisterZNodeChildChangeHandler(String path) { + zooKeeperClient.unregisterZNodeChildChangeHandler(path); + } + + private boolean registerZNodeChangeHandlerAndCheckExistence(ZNodeChangeHandler zNodeChangeHandler) + throws InterruptedException, KeeperException { + zooKeeperClient.registerZNodeChangeHandler(zNodeChangeHandler); + AsyncResponse existsResponse = retryRequestUntilConnected( + new ZooKeeperClient.ExistsRequest(zNodeChangeHandler.path(), Optional.empty())); + switch (existsResponse.getResultCode()) { + case OK: + return true; + case NONODE: + return false; + default: + throw existsResponse.resultException().get(); + } + } + + + private AsyncResponse retryRequestUntilConnected(AsyncRequest request) throws InterruptedException { + return retryRequestsUntilConnected(Sets.newHashSet(request)).get(0); + } + + private List retryRequestsUntilConnected(Set requests) throws InterruptedException { + Set remainingRequests = requests; + ArrayList responses = Lists.newArrayList(); + HashSet remainingRequestsTmp = Sets.newHashSet(); + while (!remainingRequests.isEmpty()) { + List batchResponses = zooKeeperClient.handleRequests(remainingRequests); + + // Only execute slow path if we find a response with CONNECTIONLOSS + if (batchResponses.stream() + .map(AsyncResponse::getResultCode) + .collect(Collectors.toList()) + .contains(KeeperException.Code.CONNECTIONLOSS)) { + Streams.zip( + remainingRequests.stream(), + batchResponses.stream(), + (request, response) -> { + if (response.getResultCode() == KeeperException.Code.CONNECTIONLOSS) { + remainingRequestsTmp.add(request); + } else { + responses.add(response); + } + return null; + }); + remainingRequests.clear(); + remainingRequests = remainingRequestsTmp; + if (!remainingRequests.isEmpty()) { + zooKeeperClient.waitUntilConnected(); + } + } else { + remainingRequests.clear(); + responses.addAll(batchResponses); + } + } + return responses; + } + + /** + * Get all topics marked for deletion. + * + * @return set of topics marked for deletion. + */ + public List getTopicDeletions() throws InterruptedException, KeeperException { + ZooKeeperClient.GetChildrenResponse getChildrenResponse = + (ZooKeeperClient.GetChildrenResponse) retryRequestUntilConnected( + new ZooKeeperClient.GetChildrenRequest( + getDeleteTopicsZNodePath(), + true, + Optional.empty())); + + switch (getChildrenResponse.getResultCode()) { + case OK: + return getChildrenResponse.getChildren(); + case NONODE: + return Collections.emptyList(); + default: + throw getChildrenResponse.resultException().get(); + } + } + + public static String getDeleteTopicsZNodePath() { + return "/kop/delete_topics"; + } + +} diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java new file mode 100644 index 0000000000..c743ac2dd4 --- /dev/null +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java @@ -0,0 +1,488 @@ +package io.streamnative.pulsar.handlers.kop.utils; + +import static org.apache.zookeeper.Watcher.Event.EventType.NodeChildrenChanged; +import static org.apache.zookeeper.Watcher.Event.EventType.NodeCreated; +import static org.apache.zookeeper.Watcher.Event.EventType.NodeDataChanged; +import static org.apache.zookeeper.Watcher.Event.EventType.NodeDeleted; + +import com.google.api.client.util.Lists; +import kafka.zookeeper.StateChangeHandler; +import kafka.zookeeper.ZNodeChangeHandler; +import kafka.zookeeper.ZNodeChildChangeHandler; +import kafka.zookeeper.ZooKeeperClientAuthFailedException; +import kafka.zookeeper.ZooKeeperClientExpiredException; +import kafka.zookeeper.ZooKeeperClientTimeoutException; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; +import org.apache.zookeeper.AsyncCallback; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.data.Stat; + +import java.io.IOException; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + + +@Slf4j +@Getter +public class ZooKeeperClient { + private String connectString; + private int sessionTimeoutMs; + private int connectionTimeoutMs; + private int maxInFlightRequests; + private volatile ZooKeeper zooKeeper; + + public ZooKeeperClient(String connectString, + int sessionTimeoutMs, + int connectionTimeoutMs, + int maxInFlightRequests) { + this.connectString = connectString; + this.sessionTimeoutMs = sessionTimeoutMs; + this.connectionTimeoutMs = connectionTimeoutMs; + this.maxInFlightRequests = maxInFlightRequests; + } + + private final ReentrantReadWriteLock initializationLock = new ReentrantReadWriteLock(); + private static final ReentrantLock isConnectedOrExpiredLock = new ReentrantLock(); + private static final Condition isConnectedOrExpiredCondition = isConnectedOrExpiredLock.newCondition(); + private final ConcurrentHashMap zNodeChangeHandlers = + new ConcurrentHashMap<>(); + private final ConcurrentHashMap zNodeChildChangeHandlers = + new ConcurrentHashMap<>(); + private final Semaphore inFlightRequests = new Semaphore(maxInFlightRequests); + private static final ConcurrentHashMap stateChangeHandlers = + new ConcurrentHashMap<>(); + private static final ScheduledExecutorService expiryScheduler = + new ScheduledThreadPoolExecutor(1); + + public void init() { + log.info("Initializing a new session to {}.", connectString); + try { + zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); + } catch (IOException e) { + log.error("Initializing a new session failed {}", e.getMessage()); + } + } + + public void registerZNodeChangeHandler(ZNodeChangeHandler zNodeChangeHandler) { + zNodeChangeHandlers.put(zNodeChangeHandler.path(), zNodeChangeHandler); + } + + public void unregisterZNodeChangeHandler(String path) { + zNodeChangeHandlers.remove(path); + } + + public void registerZNodeChildChangeHandler(ZNodeChildChangeHandler zNodeChildChangeHandler) { + zNodeChildChangeHandlers.put(zNodeChildChangeHandler.path(), zNodeChildChangeHandler); + } + + public void unregisterZNodeChildChangeHandler(String path) { + zNodeChildChangeHandlers.remove(path); + } + + public void registerStateChangeHandler(StateChangeHandler stateChangeHandler) { + try { + initializationLock.readLock().lock(); + if (stateChangeHandler != null) + stateChangeHandlers.put(stateChangeHandler.name(), stateChangeHandler); + } finally { + initializationLock.readLock().unlock(); + } + } + + public void unregisterStateChangeHandler(String name) { + try { + initializationLock.readLock().lock(); + stateChangeHandlers.remove(name); + } finally { + initializationLock.readLock().unlock(); + } + } + + private boolean shouldWatch(AsyncRequest request) { + switch (request.getName()) { + case "GetChildrenRequest": + return zNodeChildChangeHandlers.contains(request.getPath()); + case "ExistsRequest": + case "GetDataRequest": + return zNodeChangeHandlers.contains(request.getPath()); + default: + throw new IllegalStateException("Unexpected value: " + request.getName()); + } + } + + public void waitUntilConnected() throws InterruptedException { + try { + isConnectedOrExpiredLock.lock(); + waitUntilConnected(Long.MAX_VALUE, TimeUnit.MILLISECONDS); + } finally { + isConnectedOrExpiredLock.unlock(); + } + } + + private void waitUntilConnected(long timeout, TimeUnit timeUnit) throws InterruptedException { + log.info("Waiting until connected."); + long nanos = timeUnit.toNanos(timeout); + try { + isConnectedOrExpiredLock.lock(); + ZooKeeper.States connectionState = zooKeeper.getState(); + while (!connectionState.isConnected() && connectionState.isAlive()) { + if (nanos <= 0) { + throw new ZooKeeperClientTimeoutException( + "Timed out waiting for connection while in state: " + connectionState); + } + nanos = isConnectedOrExpiredCondition.awaitNanos(nanos); + connectionState = zooKeeper.getState(); + } + if (connectionState == ZooKeeper.States.AUTH_FAILED) { + throw new ZooKeeperClientAuthFailedException( + "Auth failed either before or while waiting for connection"); + } else if (connectionState == ZooKeeper.States.CLOSED) { + throw new ZooKeeperClientExpiredException( + "Session expired either before or while waiting for connection"); + } + log.info("Connected."); + } finally { + isConnectedOrExpiredLock.unlock(); + } + } + + public void close() { + log.info("Closing."); + try { + initializationLock.writeLock().lock(); + zNodeChangeHandlers.clear(); + zNodeChildChangeHandlers.clear(); + stateChangeHandlers.clear(); + zooKeeper.close(); + } catch (InterruptedException e) { + log.error("zookeeper close failed {}", e.getMessage()); + } finally { + initializationLock.writeLock().unlock(); + } + expiryScheduler.shutdown(); + log.info("Closed."); + } + + protected List handleRequests(Set requests) + throws InterruptedException { + if (requests.isEmpty()) { + return Lists.newArrayList(); + } else { + CountDownLatch countDownLatch = new CountDownLatch(requests.size()); + ArrayBlockingQueue responseQueue = + new ArrayBlockingQueue<>(requests.size()); + + for (AsyncRequest request : requests) { + try { + inFlightRequests.acquire(); + initializationLock.readLock().lock(); + send(request).whenComplete( + (response, throwable) -> { + responseQueue.add(response); + inFlightRequests.release(); + countDownLatch.countDown(); + }); + + } catch (Exception e) { + inFlightRequests.release(); + throw e; + } finally { + initializationLock.readLock().unlock(); + } + } + countDownLatch.await(); + + return Lists.newArrayList(responseQueue.iterator()); + } + } + + private CompletableFuture send(AsyncRequest request) { + CompletableFuture completableFuture = new CompletableFuture<>(); + + long sendTimeMs = System.currentTimeMillis(); + switch (request.getName()) { + case "ExistsRequest": + zooKeeper.exists(request.getPath(), shouldWatch(request), new AsyncCallback.StatCallback() { + @Override + public void processResult(int rc, String path, Object ctx, Stat stat) { + completableFuture.complete(new ExistsResponse( + KeeperException.Code.get(rc), + path, + ctx, + stat, + new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) + )); + + } + }, request.getCtx().orElse(null)); + break; + case "GetChildrenRequest": + zooKeeper.getChildren(request.path, shouldWatch(request), new AsyncCallback.Children2Callback() { + @Override + public void processResult(int rc, String path, Object ctx, List children, Stat stat) { + completableFuture.complete(new GetChildrenResponse( + KeeperException.Code.get(rc), + path, + ctx, + children, + stat, + new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) + )); + } + }, request.getCtx().orElse(null)); + break; + default: + throw new IllegalStateException("Unexpected value: " + request); + } + + completableFuture.complete(null); + return completableFuture; + } + + private void scheduleSessionExpiryHandler() { + expiryScheduler.schedule(() -> { + log.info("Session expired."); + reinitialize(); + }, 0, TimeUnit.MILLISECONDS); + } + + private void callBeforeInitializingSession(StateChangeHandler handler) { + try { + handler.beforeInitializingSession(); + } catch (Throwable t) { + log.error("Uncaught error in handler {}, throwable {}", handler.name(), t); + } + } + + private void callAfterInitializingSession(StateChangeHandler handler) { + try { + handler.afterInitializingSession(); + } catch (Throwable t) { + log.error("Uncaught error in handler {}, throwable {}", handler.name(), t); + } + } + + private void reinitialize() { + // Initialization callbacks are invoked outside of the lock to avoid deadlock potential since their completion + // may require additional Zookeeper requests, which will block to acquire the initialization lock + stateChangeHandlers.values().forEach( + this::callBeforeInitializingSession); + + try { + initializationLock.writeLock().lock(); + if (!zooKeeper.getState().isAlive()) { + zooKeeper.close(); + log.info("Initializing a new session to {}.", connectString); + // retry forever until ZooKeeper can be instantiated + boolean connected = false; + while (!connected) { + try { + zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); + connected = true; + } catch (Exception e) { + log.info("Error when recreating ZooKeeper, retrying after a short sleep", e); + Thread.sleep(1000); + } + } + stateChangeHandlers.values().forEach(this::callAfterInitializingSession); + } + } catch (Exception e) { + log.error("Error before recreating zookeeper when zookeeper close {}", e.getMessage()); + } finally { + initializationLock.writeLock().unlock(); + } + } + + + // package level visibility for testing only + private class ZooKeeperClientWatcher implements Watcher { + + @Override + public void process(WatchedEvent watchedEvent) { + log.debug("Received event: {}", watchedEvent); + String path = watchedEvent.getPath(); + if (path == null) { + Event.KeeperState state = watchedEvent.getState(); + try { + isConnectedOrExpiredLock.lock(); + isConnectedOrExpiredCondition.signalAll(); + } finally { + isConnectedOrExpiredLock.unlock(); + } + if (state == Event.KeeperState.AuthFailed) { + log.error("Auth failed."); + stateChangeHandlers.values().forEach(StateChangeHandler::onAuthFailure); + } else if (state == Event.KeeperState.Expired) { + scheduleSessionExpiryHandler(); + } + } else { + Event.EventType eventType = watchedEvent.getType(); + if (eventType == NodeChildrenChanged) { + zNodeChildChangeHandlers.get(path).handleChildChange(); + } else if (eventType == NodeCreated) { + zNodeChangeHandlers.get(path).handleCreation(); + } else if (eventType == NodeDeleted) { + zNodeChangeHandlers.get(path).handleDeletion(); + } else if (eventType == NodeDataChanged) { + zNodeChangeHandlers.get(path).handleDataChange(); + } + } + } + } + + @Getter + abstract static class AsyncRequest { + private final String name; + private final String path; + private final Optional ctx; + + public AsyncRequest(String path, Optional ctx, String name) { + this.path = path; + this.ctx = ctx; + this.name = name; + } + + } + + @Getter + static class ExistsRequest extends AsyncRequest { + private final String path; + private final Optional ctx; + private final static String name = "ExistsRequest"; + + public ExistsRequest(String path, Optional ctx) { + super(path, ctx, name); + this.path = path; + this.ctx = ctx; + } + } + + @Getter + static class GetChildrenRequest extends AsyncRequest { + private final String path; + private final boolean registerWatch; + private final Optional ctx; + private final static String name = "GetChildrenRequest"; + + public GetChildrenRequest(String path, boolean registerWatch, Optional ctx) { + super(path, ctx, name); + this.path = path; + this.registerWatch = registerWatch; + this.ctx = ctx; + } + } + + @Getter + abstract static class AsyncResponse { + private final KeeperException.Code resultCode; + private final String path; + private final Optional ctx; + private final Stat stat; + private final ResponseMetadata metadata; + + protected AsyncResponse(KeeperException.Code resultCode, + String path, + Optional ctx, + Stat stat, + ResponseMetadata metadata) { + this.resultCode = resultCode; + this.path = path; + this.ctx = ctx; + this.stat = stat; + this.metadata = metadata; + } + + public Optional resultException() { + if (resultCode == KeeperException.Code.OK) { + return Optional.empty(); + } + return Optional.of(KeeperException.create(resultCode, path)); + } + + public void maybeThrow() throws KeeperException { + if (resultCode != KeeperException.Code.OK) { + throw KeeperException.create(resultCode, path); + } + } + + } + + @Getter + static class ResponseMetadata { + private final long sendTimeMs; + private final long receivedTimeMs; + + public ResponseMetadata(long sendTimeMs, long receivedTimeMs) { + this.sendTimeMs = sendTimeMs; + this.receivedTimeMs = receivedTimeMs; + } + + private long responseTimeMs() { + return receivedTimeMs - sendTimeMs; + } + } + + + @Getter + static class ExistsResponse extends AsyncResponse { + private final KeeperException.Code resultCode; + private final String path; + private final Optional ctx; + private final Stat stat; + private final ResponseMetadata metadata; + + public ExistsResponse(KeeperException.Code code, + String path, + Object ctx, + Stat stat, + ResponseMetadata metadata) { + super(code, path, Optional.of(ctx), stat, metadata); + this.resultCode = code; + this.path = path; + this.ctx = Optional.of(ctx); + this.stat = stat; + this.metadata = metadata; + } + } + + @Getter + static class GetChildrenResponse extends AsyncResponse { + private final KeeperException.Code resultCode; + private final String path; + private final Optional ctx; + private final List children; + private final Stat stat; + private final ResponseMetadata metadata; + + GetChildrenResponse(KeeperException.Code resultCode, + String path, + Object ctx, + List children, + Stat stat, + ResponseMetadata metadata) { + super(resultCode, path, Optional.of(ctx), stat, metadata); + this.resultCode = resultCode; + this.path = path; + this.ctx = Optional.of(ctx); + this.children = children; + this.stat = stat; + this.metadata = metadata; + } + } +} From d1680e98202d93f291b12b3652c6bc2ab12f277c Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Mon, 6 Sep 2021 01:21:02 +0800 Subject: [PATCH 02/19] step 2 Support GroupCoordinator to delete partitions in the form of event queue --- kafka-impl/pom.xml | 6 - .../handlers/kop/KafkaProtocolHandler.java | 10 +- .../handlers/kop/KafkaRequestHandler.java | 14 +- .../group/CoordinatorEventManager.java | 85 +---- .../coordinator/group/GroupCoordinator.java | 75 +++- .../kop/coordinator/group/GroupMetadata.java | 30 ++ .../group/GroupMetadataManager.java | 29 -- .../handlers/kop/utils/KopZkClient.java | 44 ++- .../handlers/kop/utils/ZooKeeperClient.java | 325 +++++++++++++++--- 9 files changed, 434 insertions(+), 184 deletions(-) diff --git a/kafka-impl/pom.xml b/kafka-impl/pom.xml index b955955481..38531f63a0 100644 --- a/kafka-impl/pom.xml +++ b/kafka-impl/pom.xml @@ -33,12 +33,6 @@ - - org.apache.kafka - kafka_2.11 - 2.0.0-cp1 - compile - diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java index f90cd2c89f..8915f72fc7 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java @@ -292,6 +292,12 @@ public void start(BrokerService service) { ZooKeeperUtils.tryCreatePath(brokerService.pulsar().getZkClient(), kafkaConfig.getGroupIdZooKeeperPath(), new byte[0]); + ZooKeeperUtils.tryCreatePath(brokerService.pulsar().getZkClient(), + KopZkClient.getKopZNodePath(), new byte[0]); + + ZooKeeperUtils.tryCreatePath(brokerService.pulsar().getZkClient(), + KopZkClient.getDeleteTopicsZNodePath(), new byte[0]); + PulsarAdmin pulsarAdmin; try { pulsarAdmin = brokerService.getPulsar().getAdminClient(); @@ -519,7 +525,9 @@ private static KopZkClient createKopZkClient(String zkConnect) { ZooKeeperClient zooKeeperClient = new ZooKeeperClient(zkConnect, 30000, 15000, - Integer.MAX_VALUE); + 10); + + zooKeeperClient.init(); return new KopZkClient(zooKeeperClient); } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java index 26afb6c8ba..c7644937f9 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java @@ -51,6 +51,7 @@ import io.streamnative.pulsar.handlers.kop.stats.StatsLogger; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; +import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.MessageIdUtils; import io.streamnative.pulsar.handlers.kop.utils.OffsetFinder; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperUtils; @@ -2025,7 +2026,18 @@ protected void handleDeleteTopics(KafkaHeaderAndRequest deleteTopics, checkArgument(deleteTopics.getRequest() instanceof DeleteTopicsRequest); DeleteTopicsRequest request = (DeleteTopicsRequest) deleteTopics.getRequest(); Set topicsToDelete = request.topics(); - resultFuture.complete(new DeleteTopicsResponse(adminManager.deleteTopics(topicsToDelete))); + Map deleteTopicsResponse = adminManager.deleteTopics(topicsToDelete); + + // create topic znode to trigger the coordinator DeleteTopicsEvent event + deleteTopicsResponse.forEach((topic, errors) -> { + if (errors == Errors.NONE) { + ZooKeeperUtils.tryCreatePath(pulsarService.getZkClient(), + KopZkClient.getDeleteTopicsZNodePath() + "/" + topic, + new byte[0]); + } + }); + + resultFuture.complete(new DeleteTopicsResponse(deleteTopicsResponse)); } /** diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java index c5010507dd..5517fe3831 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java @@ -1,9 +1,8 @@ package io.streamnative.pulsar.handlers.kop.coordinator.group; -import java.util.concurrent.CountDownLatch; +import io.streamnative.pulsar.handlers.kop.utils.ShutdownableThread; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.locks.ReentrantLock; - import lombok.extern.slf4j.Slf4j; @Slf4j @@ -20,7 +19,13 @@ public void start() { } public void close() { - thread.shutdown(); + try { + thread.shutdown(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + log.error("Interrupted at shutting down {}", coordinatorEventThreadName); + } + } @@ -29,6 +34,7 @@ public void put(GroupCoordinator.CoordinatorEvent event) { putLock.lock(); queue.put(event); } catch (InterruptedException e) { + Thread.currentThread().interrupt(); log.error("Error put event {} to coordinator event queue {}", event, e); } finally { putLock.unlock(); @@ -45,16 +51,14 @@ public void clearAndPut(GroupCoordinator.CoordinatorEvent event) { } } - static class CoordinatorEventThread extends Thread { - private final String threadName; - private final CountDownLatch shutdownInitiated = new CountDownLatch(1); - private final CountDownLatch shutdownComplete = new CountDownLatch(1); + static class CoordinatorEventThread extends ShutdownableThread { public CoordinatorEventThread(String name) { - this.threadName = name; + super(name); } - private void doWork() { + @Override + protected void doWork() { GroupCoordinator.CoordinatorEvent event = null; try { event = queue.take(); @@ -62,71 +66,8 @@ private void doWork() { } catch (InterruptedException e) { log.error("Error processing event {}, {}", event, e); } - - } - - @Override - public void run() { - log.info("Starting"); - try { - while (isRunning()) { - doWork(); - } - } catch (Exception e) { - shutdownInitiated.countDown(); - shutdownComplete.countDown(); - log.info("Stopped"); - super.stop(); - return; - } finally { - shutdownComplete.countDown(); - } - log.info("Stopped"); } - @Override - public synchronized void start() { - setName(); - super.start(); - } - - private void setName() { - super.setName(threadName); - } - - public void shutdown() { - try { - initiateShutdown(); - awaitShutdown(); - } catch (InterruptedException e) { - log.error("shut down {} exception {}", threadName, e); - } - - } - - public boolean isShutdownComplete() { - return shutdownComplete.getCount() == 0; - } - - public boolean initiateShutdown() { - synchronized (this) { - if (isRunning()) { - log.info("Shutting down"); - shutdownInitiated.countDown(); - return true; - } - return false; - } - } - - public void awaitShutdown() throws InterruptedException { - shutdownComplete.await(); - log.info("Shutdown completed"); - } - - private boolean isRunning() { - return shutdownInitiated.getCount() != 0; - } } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java index 206233e0ba..28ff343372 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java @@ -24,11 +24,13 @@ import static org.apache.kafka.common.record.RecordBatch.NO_PRODUCER_ID; import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupOverview; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupSummary; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; +import io.streamnative.pulsar.handlers.kop.utils.KopTopic; import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.GroupKey; @@ -39,6 +41,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -52,9 +56,6 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; - -import kafka.zookeeper.ZNodeChangeHandler; -import kafka.zookeeper.ZNodeChildChangeHandler; import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.common.util.OrderedScheduler; import org.apache.kafka.common.TopicPartition; @@ -160,7 +161,7 @@ public static GroupCoordinator of( private final Time time; private final CoordinatorEventManager coordinatorEventManager; private final KopZkClient kopZkClient; - private DeletionTopicsHandler deletionTopicsHandler; + private final DeletionTopicsHandler deletionTopicsHandler; public GroupCoordinator( GroupConfig groupConfig, @@ -169,7 +170,8 @@ public GroupCoordinator( DelayedOperationPurgatory joinPurgatory, Time time, CoordinatorEventManager coordinatorEventManager, - KopZkClient kopZkClient) { + KopZkClient kopZkClient + ) { this.groupConfig = groupConfig; this.groupManager = groupManager; this.heartbeatPurgatory = heartbeatPurgatory; @@ -187,7 +189,7 @@ public void startup(boolean enableMetadataExpiration) { log.info("Starting up group coordinator."); groupManager.startup(enableMetadataExpiration); coordinatorEventManager.start(); - kopZkClient.registerZNodeChildChangeHandler(deletionTopicsHandler); + registerZNodeChildChangeHandler(); isActive.set(true); log.info("Group coordinator started."); } @@ -915,7 +917,7 @@ public KeyValue handleDescribeGroup(String groupId) { ); } - public CompletableFuture handleDeletedPartitions(List topicPartitions) { + public CompletableFuture handleDeletedPartitions(Set topicPartitions) { return groupManager.cleanGroupMetadata(groupManager.currentGroupsStream(), group -> group.removeOffsets(topicPartitions.stream()) ).thenApply(offsetsRemoved -> { @@ -1318,7 +1320,17 @@ private boolean isCoordinatorLoadInProgress(String groupId) { return groupManager.isGroupLoading(groupId); } - class DeletionTopicsHandler implements ZNodeChildChangeHandler { + private void registerZNodeChildChangeHandler() { + kopZkClient.registerZNodeChildChangeHandler(deletionTopicsHandler); + try { + // Really register ZNodeChildChange to zk. + kopZkClient.getTopicDeletions(); + } catch (InterruptedException | KeeperException e) { + e.printStackTrace(); + } + } + + class DeletionTopicsHandler implements ZooKeeperClient.ZNodeChildChangeHandler { private final CoordinatorEventManager coordinatorEventManager; public DeletionTopicsHandler(CoordinatorEventManager coordinatorEventManager) { @@ -1344,22 +1356,51 @@ class DeleteTopicsEvent implements CoordinatorEvent { @Override public void process() { -// groupManager - if (!isActive.get()) { return; } - List topicDeletions = null; try { - topicDeletions = kopZkClient.getTopicDeletions(); - log.debug("Delete topics listener fired for topics {} to be deleted", topicDeletions); + List topicsDeletions = kopZkClient.getTopicDeletions(); + + HashSet topicsFullNameDeletionsSets = Sets.newHashSet(); + HashSet kopTopicsSet = Sets.newHashSet(); + topicsDeletions.forEach(topic -> { + KopTopic kopTopic = new KopTopic(topic); + kopTopicsSet.add(kopTopic); + topicsFullNameDeletionsSets.add(kopTopic.getFullName()); + }); + + log.debug("Delete topics listener fired for topics {} to be deleted", topicsDeletions); Iterable groupMetadataIterable = groupManager.currentGroups(); - ZooKeeperClient zkClient = kopZkClient.getZooKeeperClient(); + HashSet topicPartitionsToBeDeletions = Sets.newHashSet(); - } catch (InterruptedException e) { - e.printStackTrace(); - } catch (KeeperException e) { + groupMetadataIterable.forEach(groupMetadata -> { + topicPartitionsToBeDeletions.addAll( + groupMetadata.collectPartitionsWithTopics(topicsFullNameDeletionsSets)); + }); + + Set deletedTopics = Sets.newHashSet(); + if (!topicPartitionsToBeDeletions.isEmpty()) { + handleDeletedPartitions(topicPartitionsToBeDeletions); + Set collectDeleteTopics = topicPartitionsToBeDeletions + .stream() + .map(TopicPartition::topic) + .collect(Collectors.toSet()); + + deletedTopics = kopTopicsSet.stream().filter( + kopTopic -> collectDeleteTopics.contains(kopTopic.getFullName()) + ).map(KopTopic::getOriginalName).collect(Collectors.toSet()); + + kopZkClient.deleteNodesForPaths( + KopZkClient.getDeleteTopicsZNodePath(), deletedTopics); + } + + log.info("GroupMetadata delete topics {}, no matching topics {}", + deletedTopics, Sets.difference(topicsFullNameDeletionsSets , deletedTopics)); + + } catch (Exception e) { + log.error("DeleteTopicsEvent process have an error {}", e.getMessage()); e.printStackTrace(); } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java index c70b1bbf56..d59f32f286 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java @@ -27,6 +27,8 @@ import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; + +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -588,6 +590,13 @@ public Map removeOffsets(Stream( + topicPartition, + OffsetAndMetadata.apply(0) + ); + } return new KeyValue<>( topicPartition, removedOffset.offsetAndMetadata() @@ -598,6 +607,27 @@ public Map removeOffsets(Stream collectPartitionsWithTopics(Set topics) { + HashSet topicPartitions = Sets.newHashSet(); + + topicPartitions.addAll(pendingOffsetCommits.keySet().stream().filter( + topicPartition -> topics.contains(topicPartition.topic()) + ).collect(Collectors.toSet())); + + pendingTransactionalOffsetCommits.values().stream().map(Map::keySet) + .collect(Collectors.toList()).forEach(partitionSet -> { + topicPartitions.addAll(partitionSet.stream().filter( + topicPartition -> topics.contains(topicPartition.topic())) + .collect(Collectors.toList())); + }); + + topicPartitions.addAll(offsets.keySet().stream().filter( + topicPartition -> topics.contains(topicPartition.topic()) + ).collect(Collectors.toList())); + + return topicPartitions; + } + public Map removeExpiredOffsets(long startMs) { Map expiredOffsets = offsets.entrySet().stream() .filter(e -> diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java index 159eac702f..0bdd83cc43 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadataManager.java @@ -31,7 +31,6 @@ import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.CommitRecordMetadataAndOffset; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; -import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.MessageIdUtils; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -56,9 +55,6 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; - -import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; -import kafka.zookeeper.ZNodeChangeHandler; import lombok.Data; import lombok.Getter; import lombok.experimental.Accessors; @@ -1407,29 +1403,4 @@ CompletableFuture> getOffsetsTopicReader(int partitionId) { .createAsync(); }); } - - public static class TopicChangeHandler implements ZNodeChangeHandler { - - @Override - public String path() { - return "/deletetopics"; - } - - @Override - public void handleCreation() { - ZNodeChangeHandler.super.handleCreation(); - } - - @Override - public void handleDeletion() { - ZNodeChangeHandler.super.handleDeletion(); - } - - @Override - public void handleDataChange() { - ZNodeChangeHandler.super.handleDataChange(); - } - } - - } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java index 676368ee87..9596287660 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java @@ -3,11 +3,11 @@ import com.google.api.client.util.Lists; import com.google.common.collect.Sets; import com.google.common.collect.Streams; -import kafka.zookeeper.ZNodeChangeHandler; -import kafka.zookeeper.ZNodeChildChangeHandler; +import lombok.extern.slf4j.Slf4j; import org.apache.zookeeper.KeeperException; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncRequest; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncResponse; + import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -16,6 +16,7 @@ import java.util.Set; import java.util.stream.Collectors; +@Slf4j public class KopZkClient { private final ZooKeeperClient zooKeeperClient; @@ -28,7 +29,7 @@ public ZooKeeperClient getZooKeeperClient() { return zooKeeperClient; } - public void registerZNodeChildChangeHandler(ZNodeChildChangeHandler zNodeChildChangeHandler) { + public void registerZNodeChildChangeHandler(ZooKeeperClient.ZNodeChildChangeHandler zNodeChildChangeHandler) { zooKeeperClient.registerZNodeChildChangeHandler(zNodeChildChangeHandler); } @@ -36,7 +37,7 @@ public void unregisterZNodeChildChangeHandler(String path) { zooKeeperClient.unregisterZNodeChildChangeHandler(path); } - private boolean registerZNodeChangeHandlerAndCheckExistence(ZNodeChangeHandler zNodeChangeHandler) + private boolean registerZNodeChangeHandlerAndCheckExistence(ZooKeeperClient.ZNodeChangeHandler zNodeChangeHandler) throws InterruptedException, KeeperException { zooKeeperClient.registerZNodeChangeHandler(zNodeChangeHandler); AsyncResponse existsResponse = retryRequestUntilConnected( @@ -98,10 +99,14 @@ private List retryRequestsUntilConnected(Set getTopicDeletions() throws InterruptedException, KeeperException { + return getChildren(getDeleteTopicsZNodePath()); + } + + public List getChildren(String path) throws InterruptedException, KeeperException { ZooKeeperClient.GetChildrenResponse getChildrenResponse = (ZooKeeperClient.GetChildrenResponse) retryRequestUntilConnected( new ZooKeeperClient.GetChildrenRequest( - getDeleteTopicsZNodePath(), + path, true, Optional.empty())); @@ -115,6 +120,35 @@ public List getTopicDeletions() throws InterruptedException, KeeperExcep } } + public byte[] getDataForPath(String path) throws InterruptedException, KeeperException { + ZooKeeperClient.GetDataRequest getDataRequest = + new ZooKeeperClient.GetDataRequest(path, Optional.empty()); + ZooKeeperClient.GetDataResponse getDataResponse = + (ZooKeeperClient.GetDataResponse) retryRequestUntilConnected(getDataRequest); + switch (getDataResponse.getResultCode()) { + case OK: + return getDataResponse.getData(); + case NONODE: + return new byte[0]; + default: + throw getDataResponse.resultException().get(); + } + } + + public void deleteNodesForPaths(String prePath, Set paths) throws InterruptedException { + HashSet deleteRequests = Sets.newHashSet(); + paths.forEach(path -> { + deleteRequests.add(new ZooKeeperClient.DeleteRequest( + prePath + "/" + path, -1, Optional.empty())); + }); + + retryRequestsUntilConnected(deleteRequests); + } + + public static String getKopZNodePath() { + return "/kop"; + } + public static String getDeleteTopicsZNodePath() { return "/kop/delete_topics"; } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java index c743ac2dd4..2a1ea71da4 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java @@ -6,19 +6,15 @@ import static org.apache.zookeeper.Watcher.Event.EventType.NodeDeleted; import com.google.api.client.util.Lists; -import kafka.zookeeper.StateChangeHandler; -import kafka.zookeeper.ZNodeChangeHandler; -import kafka.zookeeper.ZNodeChildChangeHandler; -import kafka.zookeeper.ZooKeeperClientAuthFailedException; -import kafka.zookeeper.ZooKeeperClientExpiredException; -import kafka.zookeeper.ZooKeeperClientTimeoutException; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.apache.zookeeper.AsyncCallback; +import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.Watcher; import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.data.ACL; import org.apache.zookeeper.data.Stat; import java.io.IOException; @@ -36,27 +32,17 @@ import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiConsumer; @Slf4j @Getter public class ZooKeeperClient { - private String connectString; - private int sessionTimeoutMs; - private int connectionTimeoutMs; + private final String connectString; + private final int sessionTimeoutMs; + private final int connectionTimeoutMs; private int maxInFlightRequests; private volatile ZooKeeper zooKeeper; - - public ZooKeeperClient(String connectString, - int sessionTimeoutMs, - int connectionTimeoutMs, - int maxInFlightRequests) { - this.connectString = connectString; - this.sessionTimeoutMs = sessionTimeoutMs; - this.connectionTimeoutMs = connectionTimeoutMs; - this.maxInFlightRequests = maxInFlightRequests; - } - private final ReentrantReadWriteLock initializationLock = new ReentrantReadWriteLock(); private static final ReentrantLock isConnectedOrExpiredLock = new ReentrantLock(); private static final Condition isConnectedOrExpiredCondition = isConnectedOrExpiredLock.newCondition(); @@ -64,33 +50,73 @@ public ZooKeeperClient(String connectString, new ConcurrentHashMap<>(); private final ConcurrentHashMap zNodeChildChangeHandlers = new ConcurrentHashMap<>(); - private final Semaphore inFlightRequests = new Semaphore(maxInFlightRequests); + private final Semaphore inFlightRequests; private static final ConcurrentHashMap stateChangeHandlers = new ConcurrentHashMap<>(); private static final ScheduledExecutorService expiryScheduler = new ScheduledThreadPoolExecutor(1); + public ZooKeeperClient(String connectString, + int sessionTimeoutMs, + int connectionTimeoutMs, + int maxInFlightRequests) { + this.connectString = connectString; + this.sessionTimeoutMs = sessionTimeoutMs; + this.connectionTimeoutMs = connectionTimeoutMs; + this.maxInFlightRequests = maxInFlightRequests; + this.inFlightRequests = new Semaphore(maxInFlightRequests); + } + public void init() { log.info("Initializing a new session to {}.", connectString); try { - zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); - } catch (IOException e) { + this.zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); + waitUntilConnected(connectionTimeoutMs, TimeUnit.MILLISECONDS); + } catch (IOException | InterruptedException e) { log.error("Initializing a new session failed {}", e.getMessage()); + close(); } } + /** + * Register the handler to ZooKeeperClient. This is just a local operation. This does not actually register a watcher. + *

+ * The watcher is only registered once the user calls handle(AsyncRequest) or handle(Seq[AsyncRequest]) + * with either a GetDataRequest or ExistsRequest. + *

+ * NOTE: zookeeper only allows registration to a nonexistent znode with ExistsRequest. + * + * @param zNodeChangeHandler the handler to register + */ public void registerZNodeChangeHandler(ZNodeChangeHandler zNodeChangeHandler) { zNodeChangeHandlers.put(zNodeChangeHandler.path(), zNodeChangeHandler); } + /** + * Unregister the handler from ZooKeeperClient. This is just a local operation. + * + * @param path the path of the handler to unregister + */ public void unregisterZNodeChangeHandler(String path) { zNodeChangeHandlers.remove(path); } + /** + * Register the handler to ZooKeeperClient. This is just a local operation. This does not actually register a watcher. + *

+ * The watcher is only registered once the user calls handle(AsyncRequest) or handle(Seq[AsyncRequest]) with a GetChildrenRequest. + * + * @param zNodeChildChangeHandler the handler to register + */ public void registerZNodeChildChangeHandler(ZNodeChildChangeHandler zNodeChildChangeHandler) { zNodeChildChangeHandlers.put(zNodeChildChangeHandler.path(), zNodeChildChangeHandler); } + /** + * Unregister the handler from ZooKeeperClient. This is just a local operation. + * + * @param path the path of the handler to unregister + */ public void unregisterZNodeChildChangeHandler(String path) { zNodeChildChangeHandlers.remove(path); } @@ -117,7 +143,7 @@ public void unregisterStateChangeHandler(String name) { private boolean shouldWatch(AsyncRequest request) { switch (request.getName()) { case "GetChildrenRequest": - return zNodeChildChangeHandlers.contains(request.getPath()); + return zNodeChildChangeHandlers.containsKey(request.getPath()); case "ExistsRequest": case "GetDataRequest": return zNodeChangeHandlers.contains(request.getPath()); @@ -165,6 +191,7 @@ private void waitUntilConnected(long timeout, TimeUnit timeUnit) throws Interrup public void close() { log.info("Closing."); try { + expiryScheduler.shutdown(); initializationLock.writeLock().lock(); zNodeChangeHandlers.clear(); zNodeChildChangeHandlers.clear(); @@ -175,7 +202,6 @@ public void close() { } finally { initializationLock.writeLock().unlock(); } - expiryScheduler.shutdown(); log.info("Closed."); } @@ -188,17 +214,17 @@ protected List handleRequests(Set requests) ArrayBlockingQueue responseQueue = new ArrayBlockingQueue<>(requests.size()); + final BiConsumer responseCallback = (response, throwable) -> { + responseQueue.add(response); + inFlightRequests.release(); + countDownLatch.countDown(); + }; + for (AsyncRequest request : requests) { try { inFlightRequests.acquire(); initializationLock.readLock().lock(); - send(request).whenComplete( - (response, throwable) -> { - responseQueue.add(response); - inFlightRequests.release(); - countDownLatch.countDown(); - }); - + send(request, responseCallback); } catch (Exception e) { inFlightRequests.release(); throw e; @@ -206,53 +232,77 @@ protected List handleRequests(Set requests) initializationLock.readLock().unlock(); } } + countDownLatch.await(); return Lists.newArrayList(responseQueue.iterator()); } } - private CompletableFuture send(AsyncRequest request) { - CompletableFuture completableFuture = new CompletableFuture<>(); + private void send(AsyncRequest request, + BiConsumer callback) { long sendTimeMs = System.currentTimeMillis(); switch (request.getName()) { case "ExistsRequest": - zooKeeper.exists(request.getPath(), shouldWatch(request), new AsyncCallback.StatCallback() { - @Override - public void processResult(int rc, String path, Object ctx, Stat stat) { - completableFuture.complete(new ExistsResponse( + zooKeeper.exists(request.getPath(), shouldWatch(request), + (rc, path, ctx, stat) -> callback.accept(new ExistsResponse( KeeperException.Code.get(rc), path, ctx, stat, new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - )); - - } - }, request.getCtx().orElse(null)); + ), null), request.getCtx()); break; case "GetChildrenRequest": - zooKeeper.getChildren(request.path, shouldWatch(request), new AsyncCallback.Children2Callback() { - @Override - public void processResult(int rc, String path, Object ctx, List children, Stat stat) { - completableFuture.complete(new GetChildrenResponse( + zooKeeper.getChildren(request.getPath(), shouldWatch(request), + (rc, path, ctx, children, stat) -> callback.accept(new GetChildrenResponse( KeeperException.Code.get(rc), path, ctx, children, stat, new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - )); - } - }, request.getCtx().orElse(null)); + ), null), request.getCtx()); + break; + case "CreateRequest": + CreateRequest createRequest = (CreateRequest) request; + zooKeeper.create(createRequest.getPath(), + createRequest.getData(), + createRequest.getAcls(), + createRequest.getCreateMode(), + (rc, path, ctx, name, stat) -> callback.accept(new CreateResponse( + KeeperException.Code.get(rc), + path, + ctx, + name, + new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) + ), null), createRequest.getCtx()); + break; + case "DeleteRequest": + DeleteRequest deleteRequest = (DeleteRequest) request; + zooKeeper.delete(deleteRequest.getPath(), deleteRequest.getVersion(), + (rc, path, ctx) -> callback.accept(new DeleteResponse( + KeeperException.Code.get(rc), + path, + ctx, + new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) + ), null), deleteRequest.getCtx()); break; + case "GetDataRequest": + zooKeeper.getData(request.getPath(), shouldWatch(request), + (rc, path, ctx, data, stat) -> callback.accept(new GetDataResponse( + KeeperException.Code.get(rc), + path, + ctx, + data, + stat, + new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) + ), null), request.getCtx()); default: throw new IllegalStateException("Unexpected value: " + request); } - completableFuture.complete(null); - return completableFuture; } private void scheduleSessionExpiryHandler() { @@ -293,7 +343,7 @@ private void reinitialize() { boolean connected = false; while (!connected) { try { - zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); + this.zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); connected = true; } catch (Exception e) { log.info("Error when recreating ZooKeeper, retrying after a short sleep", e); @@ -310,7 +360,6 @@ private void reinitialize() { } - // package level visibility for testing only private class ZooKeeperClientWatcher implements Watcher { @Override @@ -333,6 +382,7 @@ public void process(WatchedEvent watchedEvent) { } } else { Event.EventType eventType = watchedEvent.getType(); + if (eventType == NodeChildrenChanged) { zNodeChildChangeHandlers.get(path).handleChildChange(); } else if (eventType == NodeCreated) { @@ -388,6 +438,59 @@ public GetChildrenRequest(String path, boolean registerWatch, Optional ctx) { } } + @Getter + static class CreateRequest extends AsyncRequest { + private final String path; + private final byte[] data; + private final List acls; + private final CreateMode createMode; + private final Optional ctx; + private final static String name = "CreateRequest"; + + CreateRequest(String path, + byte[] data, + List acls, + CreateMode createMode, + Object ctx) { + super(path, Optional.of(createMode), name); + this.path = path; + this.data = data; + this.acls = acls; + this.createMode = createMode; + this.ctx = Optional.of(ctx); + } + } + + @Getter + static class DeleteRequest extends AsyncRequest { + private final String path; + private final int version; + private final Optional ctx; + private final static String name = "DeleteRequest"; + + DeleteRequest(String path, + int version, + Object ctx) { + super(path, Optional.of(ctx), name); + this.path = path; + this.version = version; + this.ctx = Optional.of(ctx); + } + } + + @Getter + static class GetDataRequest extends AsyncRequest { + private final String path; + private final Optional ctx; + private final static String name = "GetDataRequest"; + + GetDataRequest(String path, Object ctx) { + super(path, Optional.of(ctx), name); + this.path = path; + this.ctx = Optional.of(ctx); + } + } + @Getter abstract static class AsyncResponse { private final KeeperException.Code resultCode; @@ -485,4 +588,120 @@ static class GetChildrenResponse extends AsyncResponse { this.metadata = metadata; } } + + @Getter + static class CreateResponse extends AsyncResponse { + private final KeeperException.Code resultCode; + private final String path; + private final Optional ctx; + private final String name; + private final ResponseMetadata metadata; + + CreateResponse(KeeperException.Code resultCode, + String path, + Object ctx, + String name, + ResponseMetadata metadata) { + super(resultCode, path, Optional.of(ctx), null, metadata); + this.resultCode = resultCode; + this.path = path; + this.ctx = Optional.of(ctx); + this.name = name; + this.metadata = metadata; + } + } + + @Getter + static class DeleteResponse extends AsyncResponse { + private final KeeperException.Code resultCode; + private final String path; + private final Optional ctx; + private final ResponseMetadata metadata; + + DeleteResponse(KeeperException.Code resultCode, + String path, + Object ctx, + ResponseMetadata metadata) { + super(resultCode, path, Optional.of(ctx), null, metadata); + this.resultCode = resultCode; + this.path = path; + this.ctx = Optional.of(ctx); + this.metadata = metadata; + } + } + + @Getter + static class GetDataResponse extends AsyncResponse { + private final KeeperException.Code resultCode; + private final String path; + private final Optional ctx; + private final byte[] data; + private final Stat stat; + private final ResponseMetadata metadata; + + GetDataResponse(KeeperException.Code resultCode, + String path, + Object ctx, + byte[] data, + Stat stat, + ResponseMetadata metadata) { + super(resultCode, path, Optional.of(ctx), stat, metadata); + this.resultCode = resultCode; + this.path = path; + this.ctx = Optional.of(ctx); + this.data = data; + this.stat = stat; + this.metadata = metadata; + } + } + + public interface StateChangeHandler { + String name(); + + void beforeInitializingSession(); + + void afterInitializingSession(); + + void onAuthFailure(); + } + + public interface ZNodeChangeHandler { + String path(); + + void handleCreation(); + + void handleDeletion(); + + void handleDataChange(); + } + + public interface ZNodeChildChangeHandler { + String path(); + + void handleChildChange(); + } + + static class ZooKeeperClientException extends RuntimeException { + public ZooKeeperClientException(String message) { + super(message); + } + } + + static class ZooKeeperClientExpiredException extends ZooKeeperClientException { + public ZooKeeperClientExpiredException(String message) { + super(message); + } + } + + static class ZooKeeperClientAuthFailedException extends ZooKeeperClientException { + public ZooKeeperClientAuthFailedException(String message) { + super(message); + } + } + + static class ZooKeeperClientTimeoutException extends ZooKeeperClientException { + public ZooKeeperClientTimeoutException(String message) { + super(message); + } + } } From e27dae3605fc92641ff391e1ac3e790d1ef55f1e Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 01:12:22 +0800 Subject: [PATCH 03/19] step 3 Support KopEventManager adn the single-threaded event queue, after deleting the topic, processes the groups that have the deleted partitions, and updates the kop brokers cache information in real time. --- .../pulsar/handlers/kop/AdminManager.java | 18 ++ .../handlers/kop/KafkaChannelInitializer.java | 7 +- .../handlers/kop/KafkaProtocolHandler.java | 19 +- .../handlers/kop/KafkaRequestHandler.java | 32 +- .../pulsar/handlers/kop/KopEventManager.java | 285 ++++++++++++++++++ .../group/CoordinatorEventManager.java | 73 ----- .../coordinator/group/GroupCoordinator.java | 115 +------ .../handlers/kop/utils/KopZkClient.java | 20 +- .../handlers/kop/utils/ZooKeeperClient.java | 3 +- 9 files changed, 353 insertions(+), 219 deletions(-) create mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java delete mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java index b1ba940220..b8fa122673 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java @@ -16,21 +16,27 @@ import static io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.TopicKey; import static org.apache.kafka.common.requests.CreateTopicsRequest.TopicDetails; +import com.google.api.client.util.Sets; import io.streamnative.pulsar.handlers.kop.exceptions.KoPTopicException; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperation; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationPurgatory; import io.streamnative.pulsar.handlers.kop.utils.timer.SystemTimer; + +import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; +import org.apache.kafka.common.Node; import org.apache.kafka.common.config.ConfigResource; import org.apache.kafka.common.errors.InvalidRequestException; import org.apache.kafka.common.errors.TopicExistsException; @@ -52,6 +58,8 @@ class AdminManager { private final PulsarAdmin admin; private final int defaultNumPartitions; + private final CopyOnWriteArraySet brokersCache = new CopyOnWriteArraySet<>(); + public AdminManager(PulsarAdmin admin, KafkaServiceConfiguration conf) { this.admin = admin; @@ -221,4 +229,14 @@ public Map deleteTopics(Set topicsToDelete) { }); return result; } + + public Collection getBrokers() { + HashSet kopBrokers = Sets.newHashSet(); + kopBrokers.addAll(brokersCache); + return kopBrokers; + } + + public boolean addBrokers(Set brokers) { + return brokersCache.addAll(brokers); + } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java index 5253db2288..24d76ef6c0 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java @@ -20,7 +20,6 @@ import io.netty.handler.codec.LengthFieldBasedFrameDecoder; import io.netty.handler.codec.LengthFieldPrepender; import io.netty.handler.ssl.SslHandler; -import io.streamnative.pulsar.handlers.kop.coordinator.group.CoordinatorEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.transaction.TransactionCoordinator; import io.streamnative.pulsar.handlers.kop.stats.StatsLogger; @@ -45,8 +44,6 @@ public class KafkaChannelInitializer extends ChannelInitializer { @Getter private final GroupCoordinator groupCoordinator; @Getter - private final CoordinatorEventManager coordinatorEventManager; - @Getter private final TransactionCoordinator transactionCoordinator; private final AdminManager adminManager; @Getter @@ -62,7 +59,6 @@ public class KafkaChannelInitializer extends ChannelInitializer { public KafkaChannelInitializer(PulsarService pulsarService, KafkaServiceConfiguration kafkaConfig, GroupCoordinator groupCoordinator, - CoordinatorEventManager coordinatorEventManager, TransactionCoordinator transactionCoordinator, AdminManager adminManager, boolean enableTLS, @@ -73,7 +69,6 @@ public KafkaChannelInitializer(PulsarService pulsarService, this.pulsarService = pulsarService; this.kafkaConfig = kafkaConfig; this.groupCoordinator = groupCoordinator; - this.coordinatorEventManager = coordinatorEventManager; this.transactionCoordinator = transactionCoordinator; this.adminManager = adminManager; this.enableTls = enableTLS; @@ -98,7 +93,7 @@ protected void initChannel(SocketChannel ch) throws Exception { new LengthFieldBasedFrameDecoder(MAX_FRAME_LENGTH, 0, 4, 0, 4)); ch.pipeline().addLast("handler", new KafkaRequestHandler(pulsarService, kafkaConfig, - groupCoordinator, coordinatorEventManager, transactionCoordinator, adminManager, + groupCoordinator, transactionCoordinator, adminManager, localBrokerDataCache, enableTls, advertisedEndPoint, statsLogger)); } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java index 8915f72fc7..b8d27fb0d7 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableMap; import io.netty.channel.ChannelInitializer; import io.netty.channel.socket.SocketChannel; -import io.streamnative.pulsar.handlers.kop.coordinator.group.CoordinatorEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupConfig; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.group.OffsetConfig; @@ -99,7 +98,7 @@ public class KafkaProtocolHandler implements ProtocolHandler { @Getter private TransactionCoordinator transactionCoordinator; @Getter - private CoordinatorEventManager coordinatorEventManager; + private KopEventManager kopEventManager; @Getter private KopZkClient kopZkClient; @@ -334,11 +333,13 @@ public void start(BrokerService service) { } kopZkClient = createKopZkClient(kafkaConfig.getZookeeperServers()); - // init coordinatorEventManager - coordinatorEventManager = new CoordinatorEventManager(); // init and start group coordinator startGroupCoordinator(pulsarClient); + // init KopEventManager + kopEventManager = new KopEventManager(groupCoordinator, kopZkClient, adminManager); + kopEventManager.start(); + // and listener for Offset topics load/unload brokerService.pulsar() .getNamespaceService() @@ -392,15 +393,13 @@ public Map> newChannelIniti case PLAINTEXT: case SASL_PLAINTEXT: builder.put(endPoint.getInetAddress(), new KafkaChannelInitializer(brokerService.getPulsar(), - kafkaConfig, groupCoordinator, coordinatorEventManager, - transactionCoordinator, adminManager, false, + kafkaConfig, groupCoordinator, transactionCoordinator, adminManager, false, advertisedEndPoint, rootStatsLogger.scope(SERVER_SCOPE), localBrokerDataCache)); break; case SSL: case SASL_SSL: builder.put(endPoint.getInetAddress(), new KafkaChannelInitializer(brokerService.getPulsar(), - kafkaConfig, groupCoordinator, coordinatorEventManager, - transactionCoordinator, adminManager, true, + kafkaConfig, groupCoordinator, transactionCoordinator, adminManager, true, advertisedEndPoint, rootStatsLogger.scope(SERVER_SCOPE), localBrokerDataCache)); break; } @@ -451,9 +450,7 @@ public void startGroupCoordinator(PulsarClient pulsarClient) { SystemTimer.builder() .executorName("group-coordinator-timer") .build(), - Time.SYSTEM, - coordinatorEventManager, - kopZkClient + Time.SYSTEM ); // always enable metadata expiration this.groupCoordinator.startup(true); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java index c7644937f9..3fbe7b3ddc 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java @@ -25,13 +25,13 @@ import static org.apache.kafka.common.protocol.CommonFields.THROTTLE_TIME_MS; import static org.apache.kafka.common.requests.CreateTopicsRequest.TopicDetails; +import com.google.api.client.util.Sets; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; -import io.streamnative.pulsar.handlers.kop.coordinator.group.CoordinatorEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupOverview; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupSummary; @@ -63,6 +63,7 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -193,7 +194,6 @@ public class KafkaRequestHandler extends KafkaCommandDecoder { private final PulsarService pulsarService; private final KafkaTopicManager topicManager; private final GroupCoordinator groupCoordinator; - private final CoordinatorEventManager coordinatorEventManager; private final TransactionCoordinator transactionCoordinator; private final String clusterName; @@ -243,7 +243,6 @@ public class KafkaRequestHandler extends KafkaCommandDecoder { public KafkaRequestHandler(PulsarService pulsarService, KafkaServiceConfiguration kafkaConfig, GroupCoordinator groupCoordinator, - CoordinatorEventManager coordinatorEventManager, TransactionCoordinator transactionCoordinator, AdminManager adminManager, MetadataCache localBrokerDataCache, @@ -253,7 +252,6 @@ public KafkaRequestHandler(PulsarService pulsarService, super(statsLogger, kafkaConfig); this.pulsarService = pulsarService; this.groupCoordinator = groupCoordinator; - this.coordinatorEventManager = coordinatorEventManager; this.transactionCoordinator = transactionCoordinator; this.clusterName = kafkaConfig.getClusterName(); this.executor = pulsarService.getExecutor(); @@ -468,7 +466,9 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, // Command response for all topics List allTopicMetadata = Collections.synchronizedList(Lists.newArrayList()); - List allNodes = Collections.synchronizedList(Lists.newArrayList()); + Set allNodes = Collections.synchronizedSet(Sets.newHashSet()); + // Get all kop brokers in local cache + allNodes.addAll(adminManager.getBrokers()); List topics = metadataRequest.topics(); // topics in format : persistent://%s/%s/abc-partition-x, will be grouped by as: @@ -651,11 +651,11 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, ctx.channel(), metadataHar.getHeader(), e); allNodes.add(newSelfNode()); MetadataResponse finalResponse = - new MetadataResponse( - allNodes, - clusterName, - controllerId, - Collections.emptyList()); + new MetadataResponse( + Lists.newArrayList(allNodes), + clusterName, + controllerId, + Collections.emptyList()); resultFuture.complete(finalResponse); return; } @@ -666,11 +666,11 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, // no topic partitions added, return now. allNodes.add(newSelfNode()); MetadataResponse finalResponse = - new MetadataResponse( - allNodes, - clusterName, - controllerId, - allTopicMetadata); + new MetadataResponse( + Lists.newArrayList(allNodes), + clusterName, + controllerId, + allTopicMetadata); resultFuture.complete(finalResponse); return; } @@ -734,7 +734,7 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, // TODO: confirm right value for controller_id MetadataResponse finalResponse = new MetadataResponse( - allNodes, + Lists.newArrayList(allNodes), clusterName, controllerId, allTopicMetadata); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java new file mode 100644 index 0000000000..e3ce73e86f --- /dev/null +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -0,0 +1,285 @@ +package io.streamnative.pulsar.handlers.kop; + +import com.google.common.collect.Sets; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; +import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata; +import io.streamnative.pulsar.handlers.kop.utils.KopTopic; +import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; +import io.streamnative.pulsar.handlers.kop.utils.ShutdownableThread; +import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; +import lombok.extern.slf4j.Slf4j; +import org.apache.kafka.common.Node; +import org.apache.kafka.common.TopicPartition; +import org.apache.pulsar.common.util.Murmur3_32Hash; +import org.apache.zookeeper.KeeperException; + +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.locks.ReentrantLock; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import static com.google.common.base.Preconditions.checkState; +import static java.nio.charset.StandardCharsets.UTF_8; + +@Slf4j +public class KopEventManager { + private static final String END_POINT_SEPARATOR = ","; + private static final String REGEX = "^(.*)://\\[?([0-9a-zA-Z\\-%._:]*)\\]?:(-?[0-9]+)"; + private static final Pattern PATTERN = Pattern.compile(REGEX); + + private static final String kopEventThreadName = "kop-event-thread"; + private final ReentrantLock putLock = new ReentrantLock(); + private static final LinkedBlockingQueue queue = + new LinkedBlockingQueue<>(); + private final KopEventThread thread = + new KopEventThread(kopEventThreadName); + private final GroupCoordinator coordinator; + private final KopZkClient kopZkClient; + private final AdminManager adminManager; + private final DeletionTopicsHandler deletionTopicsHandler; + private final BrokersChangeHandler brokersChangeHandler; + + public KopEventManager(GroupCoordinator coordinator, + KopZkClient kopZkClient, + AdminManager adminManager) { + this.coordinator = coordinator; + this.kopZkClient = kopZkClient; + this.adminManager = adminManager; + this.deletionTopicsHandler = new DeletionTopicsHandler(this); + this.brokersChangeHandler = new BrokersChangeHandler(this); + } + + public void start() { + registerZNodeChildChangeHandler(); + thread.start(); + } + + public void close() { + try { + thread.shutdown(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + log.error("Interrupted at shutting down {}", kopEventThreadName); + } + + } + + + public void put(KopEvent event) { + try { + putLock.lock(); + queue.put(event); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + log.error("Error put event {} to coordinator event queue {}", event, e); + } finally { + putLock.unlock(); + } + } + + public void clearAndPut(KopEvent event) { + try { + putLock.lock(); + queue.clear(); + put(event); + } finally { + putLock.unlock(); + } + } + + static class KopEventThread extends ShutdownableThread { + + public KopEventThread(String name) { + super(name); + } + + @Override + protected void doWork() { + KopEvent event = null; + try { + event = queue.take(); + event.process(); + } catch (InterruptedException e) { + log.error("Error processing event {}, {}", event, e); + } + } + + } + + private void registerZNodeChildChangeHandler() { + kopZkClient.registerZNodeChildChangeHandler(deletionTopicsHandler); + kopZkClient.registerZNodeChildChangeHandler(brokersChangeHandler); + try { + // Really register ZNodeChildChange to zk. + kopZkClient.getTopicDeletions(); + // init local kop brokers cache + getBrokers(kopZkClient.getBrokers()); + } catch (InterruptedException | KeeperException e) { + log.error("registerZNodeChildChangeHandler have failed with an error {}", e.getMessage()); + e.printStackTrace(); + } + } + + private void getBrokers(List pulsarBrokers) { + HashSet kopBrokers = Sets.newHashSet(); + pulsarBrokers.forEach(broker -> { + try { + byte[] brokerInfo = + kopZkClient.getDataForPath(KopZkClient.getBrokersChangeZNodePath() + "/" + broker); + JsonObject jsonObject = parseJsonObject(new String(brokerInfo)); + JsonObject protocols = jsonObject.getAsJsonObject("protocols"); + JsonElement element = protocols.get("kafka"); + + if (element != null) { + String kopBrokerStr = element.getAsString(); + Node kopNode = getNode(kopBrokerStr); + kopBrokers.add(kopNode); + } + } catch (Exception e) { + log.error("Get broker {} ZNode data failed which have an error {}", broker, e.getMessage()); + e.printStackTrace(); + } + }); + Collection oldKopBrokers = adminManager.getBrokers(); + adminManager.addBrokers(kopBrokers); + log.info("Refresh kop brokers new cache {}, old brokers cache {}", + adminManager.getBrokers(), oldKopBrokers); + } + + private JsonObject parseJsonObject(String info) { + JsonParser parser = new JsonParser(); + return parser.parse(info).getAsJsonObject(); + } + + private Node getNode(String kopBrokerStr) { + final String errorMessage = "kopBrokerStr " + kopBrokerStr + " is invalid"; + final Matcher matcher = PATTERN.matcher(kopBrokerStr); + checkState(matcher.find(), errorMessage); + checkState(matcher.groupCount() == 3, errorMessage); + String host = matcher.group(2); + String port = matcher.group(3); + + return new Node( + Murmur3_32Hash.getInstance().makeHash((host + port).getBytes(UTF_8)), + host, + Integer.parseInt(port)); + } + + class DeletionTopicsHandler implements ZooKeeperClient.ZNodeChildChangeHandler { + private final KopEventManager kopEventManager; + + public DeletionTopicsHandler(KopEventManager kopEventManager) { + this.kopEventManager = kopEventManager; + } + + @Override + public String path() { + return KopZkClient.getDeleteTopicsZNodePath(); + } + + @Override + public void handleChildChange() { + kopEventManager.put(new DeleteTopicsEvent()); + } + } + + class BrokersChangeHandler implements ZooKeeperClient.ZNodeChildChangeHandler { + private final KopEventManager kopEventManager; + + public BrokersChangeHandler(KopEventManager kopEventManager) { + this.kopEventManager = kopEventManager; + } + + @Override + public String path() { + return KopZkClient.getBrokersChangeZNodePath(); + } + + @Override + public void handleChildChange() { + kopEventManager.put(new BrokersChangeEvent()); + } + + } + + + interface KopEvent { + void process(); + } + + class DeleteTopicsEvent implements KopEvent { + + @Override + public void process() { + if (!coordinator.isActive()) { + return; + } + + try { + List topicsDeletions = kopZkClient.getTopicDeletions(); + + HashSet topicsFullNameDeletionsSets = Sets.newHashSet(); + HashSet kopTopicsSet = Sets.newHashSet(); + topicsDeletions.forEach(topic -> { + KopTopic kopTopic = new KopTopic(topic); + kopTopicsSet.add(kopTopic); + topicsFullNameDeletionsSets.add(kopTopic.getFullName()); + }); + + log.debug("Delete topics listener fired for topics {} to be deleted", topicsDeletions); + Iterable groupMetadataIterable = coordinator.getGroupManager().currentGroups(); + HashSet topicPartitionsToBeDeletions = Sets.newHashSet(); + + groupMetadataIterable.forEach(groupMetadata -> { + topicPartitionsToBeDeletions.addAll( + groupMetadata.collectPartitionsWithTopics(topicsFullNameDeletionsSets)); + }); + + Set deletedTopics = Sets.newHashSet(); + if (!topicPartitionsToBeDeletions.isEmpty()) { + coordinator.handleDeletedPartitions(topicPartitionsToBeDeletions); + Set collectDeleteTopics = topicPartitionsToBeDeletions + .stream() + .map(TopicPartition::topic) + .collect(Collectors.toSet()); + + deletedTopics = kopTopicsSet.stream().filter( + kopTopic -> collectDeleteTopics.contains(kopTopic.getFullName()) + ).map(KopTopic::getOriginalName).collect(Collectors.toSet()); + + kopZkClient.deleteNodesForPaths( + KopZkClient.getDeleteTopicsZNodePath(), deletedTopics); + } + + log.info("GroupMetadata delete topics {}, no matching topics {}", + deletedTopics, Sets.difference(topicsFullNameDeletionsSets, deletedTopics)); + + } catch (Exception e) { + log.error("DeleteTopicsEvent process have an error {}", e.getMessage()); + e.printStackTrace(); + } + } + } + + class BrokersChangeEvent implements KopEvent { + @Override + public void process() { + try { + getBrokers(kopZkClient.getBrokers()); + } catch (Exception e) { + log.error("BrokersChangeEvent process have an error {}", e.getMessage()); + e.printStackTrace(); + } + } + } + +} diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java deleted file mode 100644 index 5517fe3831..0000000000 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/CoordinatorEventManager.java +++ /dev/null @@ -1,73 +0,0 @@ -package io.streamnative.pulsar.handlers.kop.coordinator.group; - -import io.streamnative.pulsar.handlers.kop.utils.ShutdownableThread; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.locks.ReentrantLock; -import lombok.extern.slf4j.Slf4j; - -@Slf4j -public class CoordinatorEventManager { - private static final String coordinatorEventThreadName = "coordinator-event-thread"; - private final ReentrantLock putLock = new ReentrantLock(); - private static final LinkedBlockingQueue queue = - new LinkedBlockingQueue<>(); - private final CoordinatorEventThread thread = - new CoordinatorEventThread(coordinatorEventThreadName); - - public void start() { - thread.start(); - } - - public void close() { - try { - thread.shutdown(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - log.error("Interrupted at shutting down {}", coordinatorEventThreadName); - } - - } - - - public void put(GroupCoordinator.CoordinatorEvent event) { - try { - putLock.lock(); - queue.put(event); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - log.error("Error put event {} to coordinator event queue {}", event, e); - } finally { - putLock.unlock(); - } - } - - public void clearAndPut(GroupCoordinator.CoordinatorEvent event) { - try { - putLock.lock(); - queue.clear(); - put(event); - } finally { - putLock.unlock(); - } - } - - static class CoordinatorEventThread extends ShutdownableThread { - - public CoordinatorEventThread(String name) { - super(name); - } - - @Override - protected void doWork() { - GroupCoordinator.CoordinatorEvent event = null; - try { - event = queue.take(); - event.process(); - } catch (InterruptedException e) { - log.error("Error processing event {}, {}", event, e); - } - } - - } - -} diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java index 28ff343372..627ac3e718 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java @@ -24,15 +24,13 @@ import static org.apache.kafka.common.record.RecordBatch.NO_PRODUCER_ID; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import io.streamnative.pulsar.handlers.kop.KopEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupOverview; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupSummary; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; -import io.streamnative.pulsar.handlers.kop.utils.KopTopic; import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; -import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.GroupKey; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.MemberKey; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationPurgatory; @@ -41,8 +39,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -76,7 +72,6 @@ import org.apache.pulsar.client.impl.ReaderBuilderImpl; import org.apache.pulsar.common.schema.KeyValue; import org.apache.pulsar.common.util.FutureUtil; -import org.apache.zookeeper.KeeperException; /** * Group coordinator. @@ -89,9 +84,7 @@ public static GroupCoordinator of( GroupConfig groupConfig, OffsetConfig offsetConfig, Timer timer, - Time time, - CoordinatorEventManager coordinatorEventManager, - KopZkClient kopZkClient + Time time ) { ScheduledExecutorService coordinatorExecutor = OrderedScheduler.newSchedulerBuilder() .name("group-coordinator-executor") @@ -128,9 +121,7 @@ public static GroupCoordinator of( metadataManager, heartbeatPurgatory, joinPurgatory, - time, - coordinatorEventManager, - kopZkClient + time ); } @@ -159,27 +150,19 @@ public static GroupCoordinator of( private final DelayedOperationPurgatory heartbeatPurgatory; private final DelayedOperationPurgatory joinPurgatory; private final Time time; - private final CoordinatorEventManager coordinatorEventManager; - private final KopZkClient kopZkClient; - private final DeletionTopicsHandler deletionTopicsHandler; public GroupCoordinator( GroupConfig groupConfig, GroupMetadataManager groupManager, DelayedOperationPurgatory heartbeatPurgatory, DelayedOperationPurgatory joinPurgatory, - Time time, - CoordinatorEventManager coordinatorEventManager, - KopZkClient kopZkClient + Time time ) { this.groupConfig = groupConfig; this.groupManager = groupManager; this.heartbeatPurgatory = heartbeatPurgatory; this.joinPurgatory = joinPurgatory; this.time = time; - this.coordinatorEventManager = coordinatorEventManager; - this.kopZkClient = kopZkClient; - this.deletionTopicsHandler = new DeletionTopicsHandler(coordinatorEventManager); } /** @@ -188,8 +171,6 @@ public GroupCoordinator( public void startup(boolean enableMetadataExpiration) { log.info("Starting up group coordinator."); groupManager.startup(enableMetadataExpiration); - coordinatorEventManager.start(); - registerZNodeChildChangeHandler(); isActive.set(true); log.info("Group coordinator started."); } @@ -202,7 +183,6 @@ public void shutdown() { log.info("Shutting down group coordinator ..."); isActive.set(false); groupManager.shutdown(); - coordinatorEventManager.close(); heartbeatPurgatory.shutdown(); joinPurgatory.shutdown(); log.info("Shutdown group coordinator completely."); @@ -1320,90 +1300,7 @@ private boolean isCoordinatorLoadInProgress(String groupId) { return groupManager.isGroupLoading(groupId); } - private void registerZNodeChildChangeHandler() { - kopZkClient.registerZNodeChildChangeHandler(deletionTopicsHandler); - try { - // Really register ZNodeChildChange to zk. - kopZkClient.getTopicDeletions(); - } catch (InterruptedException | KeeperException e) { - e.printStackTrace(); - } - } - - class DeletionTopicsHandler implements ZooKeeperClient.ZNodeChildChangeHandler { - private final CoordinatorEventManager coordinatorEventManager; - - public DeletionTopicsHandler(CoordinatorEventManager coordinatorEventManager) { - this.coordinatorEventManager = coordinatorEventManager; - } - - @Override - public String path() { - return KopZkClient.getDeleteTopicsZNodePath(); - } - - @Override - public void handleChildChange() { - coordinatorEventManager.put(new DeleteTopicsEvent()); - } - } - - interface CoordinatorEvent { - void process(); - } - - class DeleteTopicsEvent implements CoordinatorEvent { - - @Override - public void process() { - if (!isActive.get()) { - return; - } - - try { - List topicsDeletions = kopZkClient.getTopicDeletions(); - - HashSet topicsFullNameDeletionsSets = Sets.newHashSet(); - HashSet kopTopicsSet = Sets.newHashSet(); - topicsDeletions.forEach(topic -> { - KopTopic kopTopic = new KopTopic(topic); - kopTopicsSet.add(kopTopic); - topicsFullNameDeletionsSets.add(kopTopic.getFullName()); - }); - - log.debug("Delete topics listener fired for topics {} to be deleted", topicsDeletions); - Iterable groupMetadataIterable = groupManager.currentGroups(); - HashSet topicPartitionsToBeDeletions = Sets.newHashSet(); - - groupMetadataIterable.forEach(groupMetadata -> { - topicPartitionsToBeDeletions.addAll( - groupMetadata.collectPartitionsWithTopics(topicsFullNameDeletionsSets)); - }); - - Set deletedTopics = Sets.newHashSet(); - if (!topicPartitionsToBeDeletions.isEmpty()) { - handleDeletedPartitions(topicPartitionsToBeDeletions); - Set collectDeleteTopics = topicPartitionsToBeDeletions - .stream() - .map(TopicPartition::topic) - .collect(Collectors.toSet()); - - deletedTopics = kopTopicsSet.stream().filter( - kopTopic -> collectDeleteTopics.contains(kopTopic.getFullName()) - ).map(KopTopic::getOriginalName).collect(Collectors.toSet()); - - kopZkClient.deleteNodesForPaths( - KopZkClient.getDeleteTopicsZNodePath(), deletedTopics); - } - - log.info("GroupMetadata delete topics {}, no matching topics {}", - deletedTopics, Sets.difference(topicsFullNameDeletionsSets , deletedTopics)); - - } catch (Exception e) { - log.error("DeleteTopicsEvent process have an error {}", e.getMessage()); - e.printStackTrace(); - } - } + public boolean isActive() { + return isActive.get(); } - } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java index 9596287660..5129211eb5 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java @@ -4,6 +4,7 @@ import com.google.common.collect.Sets; import com.google.common.collect.Streams; import lombok.extern.slf4j.Slf4j; +import org.apache.pulsar.broker.loadbalance.LoadManager; import org.apache.zookeeper.KeeperException; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncRequest; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncResponse; @@ -94,14 +95,23 @@ private List retryRequestsUntilConnected(Set getTopicDeletions() throws InterruptedException, KeeperException { return getChildren(getDeleteTopicsZNodePath()); } + /** + * Get all brokers. + * + * @return list of all pulsar brokers. + */ + public List getBrokers() throws InterruptedException, KeeperException { + return getChildren(getBrokersChangeZNodePath()); + } + public List getChildren(String path) throws InterruptedException, KeeperException { ZooKeeperClient.GetChildrenResponse getChildrenResponse = (ZooKeeperClient.GetChildrenResponse) retryRequestUntilConnected( @@ -150,7 +160,11 @@ public static String getKopZNodePath() { } public static String getDeleteTopicsZNodePath() { - return "/kop/delete_topics"; + return getKopZNodePath() + "/delete_topics"; + } + + public static String getBrokersChangeZNodePath() { + return LoadManager.LOADBALANCE_BROKERS_ROOT; } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java index 2a1ea71da4..09db31d864 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java @@ -146,7 +146,7 @@ private boolean shouldWatch(AsyncRequest request) { return zNodeChildChangeHandlers.containsKey(request.getPath()); case "ExistsRequest": case "GetDataRequest": - return zNodeChangeHandlers.contains(request.getPath()); + return zNodeChangeHandlers.containsKey(request.getPath()); default: throw new IllegalStateException("Unexpected value: " + request.getName()); } @@ -299,6 +299,7 @@ private void send(AsyncRequest request, stat, new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) ), null), request.getCtx()); + break; default: throw new IllegalStateException("Unexpected value: " + request); } From 441bf0f87e8f62fd5deca9f58e995f4f07bbb4a1 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 02:01:17 +0800 Subject: [PATCH 04/19] fix checkstyle error --- .../pulsar/handlers/kop/AdminManager.java | 3 +- .../handlers/kop/KafkaRequestHandler.java | 3 +- .../pulsar/handlers/kop/KopEventManager.java | 31 +++++--- .../coordinator/group/GroupCoordinator.java | 2 - .../kop/coordinator/group/GroupMetadata.java | 2 - .../handlers/kop/utils/KopZkClient.java | 25 +++++-- .../handlers/kop/utils/ZooKeeperClient.java | 74 +++++++++++-------- 7 files changed, 87 insertions(+), 53 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java index b8fa122673..5ddee6d94f 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java @@ -16,13 +16,12 @@ import static io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.TopicKey; import static org.apache.kafka.common.requests.CreateTopicsRequest.TopicDetails; -import com.google.api.client.util.Sets; +import com.google.common.collect.Sets; import io.streamnative.pulsar.handlers.kop.exceptions.KoPTopicException; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperation; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationPurgatory; import io.streamnative.pulsar.handlers.kop.utils.timer.SystemTimer; - import java.util.Collection; import java.util.Collections; import java.util.HashSet; diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java index 3fbe7b3ddc..f6a1886748 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java @@ -25,10 +25,10 @@ import static org.apache.kafka.common.protocol.CommonFields.THROTTLE_TIME_MS; import static org.apache.kafka.common.requests.CreateTopicsRequest.TopicDetails; -import com.google.api.client.util.Sets; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; @@ -63,7 +63,6 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index e3ce73e86f..000daa8f7a 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -1,5 +1,21 @@ +/** + * Licensed 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 io.streamnative.pulsar.handlers.kop; +import static com.google.common.base.Preconditions.checkState; +import static java.nio.charset.StandardCharsets.UTF_8; + import com.google.common.collect.Sets; import com.google.gson.JsonElement; import com.google.gson.JsonObject; @@ -10,13 +26,6 @@ import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.ShutdownableThread; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; -import lombok.extern.slf4j.Slf4j; -import org.apache.kafka.common.Node; -import org.apache.kafka.common.TopicPartition; -import org.apache.pulsar.common.util.Murmur3_32Hash; -import org.apache.zookeeper.KeeperException; - -import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -26,9 +35,11 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; - -import static com.google.common.base.Preconditions.checkState; -import static java.nio.charset.StandardCharsets.UTF_8; +import lombok.extern.slf4j.Slf4j; +import org.apache.kafka.common.Node; +import org.apache.kafka.common.TopicPartition; +import org.apache.pulsar.common.util.Murmur3_32Hash; +import org.apache.zookeeper.KeeperException; @Slf4j public class KopEventManager { diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java index 627ac3e718..982a03c40a 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupCoordinator.java @@ -25,12 +25,10 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import io.streamnative.pulsar.handlers.kop.KopEventManager; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupOverview; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata.GroupSummary; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; -import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.GroupKey; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.MemberKey; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationPurgatory; diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java index d59f32f286..97706fee35 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java @@ -27,8 +27,6 @@ import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; - -import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java index 5129211eb5..eb7d99e0a0 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java @@ -1,14 +1,23 @@ +/** + * Licensed 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 io.streamnative.pulsar.handlers.kop.utils; -import com.google.api.client.util.Lists; +import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.common.collect.Streams; -import lombok.extern.slf4j.Slf4j; -import org.apache.pulsar.broker.loadbalance.LoadManager; -import org.apache.zookeeper.KeeperException; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncRequest; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncResponse; - import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -16,6 +25,9 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; +import org.apache.pulsar.broker.loadbalance.LoadManager; +import org.apache.zookeeper.KeeperException; @Slf4j public class KopZkClient { @@ -58,7 +70,8 @@ private AsyncResponse retryRequestUntilConnected(AsyncRequest request) throws In return retryRequestsUntilConnected(Sets.newHashSet(request)).get(0); } - private List retryRequestsUntilConnected(Set requests) throws InterruptedException { + private List retryRequestsUntilConnected(Set requests) + throws InterruptedException { Set remainingRequests = requests; ArrayList responses = Lists.newArrayList(); HashSet remainingRequestsTmp = Sets.newHashSet(); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java index 09db31d864..e938d99147 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java @@ -1,3 +1,16 @@ +/** + * Licensed 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 io.streamnative.pulsar.handlers.kop.utils; import static org.apache.zookeeper.Watcher.Event.EventType.NodeChildrenChanged; @@ -5,24 +18,12 @@ import static org.apache.zookeeper.Watcher.Event.EventType.NodeDataChanged; import static org.apache.zookeeper.Watcher.Event.EventType.NodeDeleted; -import com.google.api.client.util.Lists; -import lombok.Getter; -import lombok.extern.slf4j.Slf4j; -import org.apache.zookeeper.AsyncCallback; -import org.apache.zookeeper.CreateMode; -import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.WatchedEvent; -import org.apache.zookeeper.Watcher; -import org.apache.zookeeper.ZooKeeper; -import org.apache.zookeeper.data.ACL; -import org.apache.zookeeper.data.Stat; - +import com.google.common.collect.Lists; import java.io.IOException; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ScheduledExecutorService; @@ -33,6 +34,15 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.BiConsumer; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Stat; @Slf4j @@ -79,10 +89,12 @@ public void init() { } /** - * Register the handler to ZooKeeperClient. This is just a local operation. This does not actually register a watcher. + * Register the handler to ZooKeeperClient. + * This is just a local operation. + * This does not actually register a watcher. *

- * The watcher is only registered once the user calls handle(AsyncRequest) or handle(Seq[AsyncRequest]) - * with either a GetDataRequest or ExistsRequest. + * The watcher is only registered once the user calls handle(AsyncRequest) + * or handle(Seq[AsyncRequest]) with either a GetDataRequest or ExistsRequest. *

* NOTE: zookeeper only allows registration to a nonexistent znode with ExistsRequest. * @@ -102,9 +114,12 @@ public void unregisterZNodeChangeHandler(String path) { } /** - * Register the handler to ZooKeeperClient. This is just a local operation. This does not actually register a watcher. + * Register the handler to ZooKeeperClient. + * This is just a local operation. + * This does not actually register a watcher. *

- * The watcher is only registered once the user calls handle(AsyncRequest) or handle(Seq[AsyncRequest]) with a GetChildrenRequest. + * The watcher is only registered once the user calls handle(AsyncRequest) + * or handle(Seq[AsyncRequest]) with a GetChildrenRequest. * * @param zNodeChildChangeHandler the handler to register */ @@ -124,8 +139,9 @@ public void unregisterZNodeChildChangeHandler(String path) { public void registerStateChangeHandler(StateChangeHandler stateChangeHandler) { try { initializationLock.readLock().lock(); - if (stateChangeHandler != null) + if (stateChangeHandler != null) { stateChangeHandlers.put(stateChangeHandler.name(), stateChangeHandler); + } } finally { initializationLock.readLock().unlock(); } @@ -413,12 +429,12 @@ public AsyncRequest(String path, Optional ctx, String name) { @Getter static class ExistsRequest extends AsyncRequest { + private final String name = "ExistsRequest"; private final String path; private final Optional ctx; - private final static String name = "ExistsRequest"; public ExistsRequest(String path, Optional ctx) { - super(path, ctx, name); + super(path, ctx, "ExistsRequest"); this.path = path; this.ctx = ctx; } @@ -426,13 +442,13 @@ public ExistsRequest(String path, Optional ctx) { @Getter static class GetChildrenRequest extends AsyncRequest { + private final String name = "GetChildrenRequest"; private final String path; private final boolean registerWatch; private final Optional ctx; - private final static String name = "GetChildrenRequest"; public GetChildrenRequest(String path, boolean registerWatch, Optional ctx) { - super(path, ctx, name); + super(path, ctx, "GetChildrenRequest"); this.path = path; this.registerWatch = registerWatch; this.ctx = ctx; @@ -441,19 +457,19 @@ public GetChildrenRequest(String path, boolean registerWatch, Optional ctx) { @Getter static class CreateRequest extends AsyncRequest { + private final String name = "CreateRequest"; private final String path; private final byte[] data; private final List acls; private final CreateMode createMode; private final Optional ctx; - private final static String name = "CreateRequest"; CreateRequest(String path, byte[] data, List acls, CreateMode createMode, Object ctx) { - super(path, Optional.of(createMode), name); + super(path, Optional.of(createMode), "CreateRequest"); this.path = path; this.data = data; this.acls = acls; @@ -464,15 +480,15 @@ static class CreateRequest extends AsyncRequest { @Getter static class DeleteRequest extends AsyncRequest { + private final String name = "DeleteRequest"; private final String path; private final int version; private final Optional ctx; - private final static String name = "DeleteRequest"; DeleteRequest(String path, int version, Object ctx) { - super(path, Optional.of(ctx), name); + super(path, Optional.of(ctx), "DeleteRequest"); this.path = path; this.version = version; this.ctx = Optional.of(ctx); @@ -481,12 +497,12 @@ static class DeleteRequest extends AsyncRequest { @Getter static class GetDataRequest extends AsyncRequest { + private final String name = "GetDataRequest"; private final String path; private final Optional ctx; - private final static String name = "GetDataRequest"; GetDataRequest(String path, Object ctx) { - super(path, Optional.of(ctx), name); + super(path, Optional.of(ctx), "GetDataRequest"); this.path = path; this.ctx = Optional.of(ctx); } From e7e57c14e0e32d8623a462745331aeaeebfd8d07 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 16:15:56 +0800 Subject: [PATCH 05/19] Use MetadataStore to replace Zk client --- .../pulsar/handlers/kop/AdminManager.java | 19 +- .../handlers/kop/ChildChangeHandler.java | 57 ++ .../handlers/kop/KafkaProtocolHandler.java | 25 +- .../handlers/kop/KafkaRequestHandler.java | 3 +- .../pulsar/handlers/kop/KopEventManager.java | 142 ++-- .../handlers/kop/utils/KopZkClient.java | 183 ----- .../handlers/kop/utils/ZooKeeperClient.java | 724 ------------------ 7 files changed, 137 insertions(+), 1016 deletions(-) create mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java delete mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java delete mode 100644 kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java index 5ddee6d94f..dd1c630b68 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java @@ -16,7 +16,6 @@ import static io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperationKey.TopicKey; import static org.apache.kafka.common.requests.CreateTopicsRequest.TopicDetails; -import com.google.common.collect.Sets; import io.streamnative.pulsar.handlers.kop.exceptions.KoPTopicException; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; import io.streamnative.pulsar.handlers.kop.utils.delayed.DelayedOperation; @@ -31,8 +30,8 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.apache.kafka.common.Node; @@ -57,7 +56,8 @@ class AdminManager { private final PulsarAdmin admin; private final int defaultNumPartitions; - private final CopyOnWriteArraySet brokersCache = new CopyOnWriteArraySet<>(); + private volatile Set brokersCache = new HashSet<>(); + private final ReentrantReadWriteLock brokersCacheLock = new ReentrantReadWriteLock(); public AdminManager(PulsarAdmin admin, KafkaServiceConfiguration conf) { @@ -230,12 +230,15 @@ public Map deleteTopics(Set topicsToDelete) { } public Collection getBrokers() { - HashSet kopBrokers = Sets.newHashSet(); - kopBrokers.addAll(brokersCache); - return kopBrokers; + return brokersCache; } - public boolean addBrokers(Set brokers) { - return brokersCache.addAll(brokers); + public void addBrokers(Set brokers) { + try { + brokersCacheLock.writeLock().lock(); + this.brokersCache = brokers; + } finally { + brokersCacheLock.writeLock().unlock(); + } } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java new file mode 100644 index 0000000000..94bc56dbbf --- /dev/null +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java @@ -0,0 +1,57 @@ +/** + * Licensed 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 io.streamnative.pulsar.handlers.kop; + +public interface ChildChangeHandler { + String path(); + + void handleChildChange(); +} + +class DeletionTopicsHandler implements ChildChangeHandler { + private final KopEventManager kopEventManager; + + public DeletionTopicsHandler(KopEventManager kopEventManager) { + this.kopEventManager = kopEventManager; + } + + @Override + public String path() { + return KopEventManager.getBrokersChangePath(); + } + + @Override + public void handleChildChange() { + kopEventManager.put(new KopEventManager.DeleteTopicsEvent()); + } +} + +class BrokersChangeHandler implements ChildChangeHandler { + private final KopEventManager kopEventManager; + + public BrokersChangeHandler(KopEventManager kopEventManager) { + this.kopEventManager = kopEventManager; + } + + @Override + public String path() { + return KopEventManager.getBrokersChangePath(); + } + + @Override + public void handleChildChange() { + kopEventManager.put(new KopEventManager.BrokersChangeEvent()); + } + +} diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java index b8d27fb0d7..7d54f2d2e9 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaProtocolHandler.java @@ -30,9 +30,7 @@ import io.streamnative.pulsar.handlers.kop.stats.StatsLogger; import io.streamnative.pulsar.handlers.kop.utils.ConfigurationUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; -import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.MetadataUtils; -import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperUtils; import io.streamnative.pulsar.handlers.kop.utils.timer.SystemTimer; import java.net.InetSocketAddress; @@ -99,8 +97,6 @@ public class KafkaProtocolHandler implements ProtocolHandler { private TransactionCoordinator transactionCoordinator; @Getter private KopEventManager kopEventManager; - @Getter - private KopZkClient kopZkClient; /** * Listener for the changing of topic that stores offsets of consumer group. @@ -292,10 +288,10 @@ public void start(BrokerService service) { kafkaConfig.getGroupIdZooKeeperPath(), new byte[0]); ZooKeeperUtils.tryCreatePath(brokerService.pulsar().getZkClient(), - KopZkClient.getKopZNodePath(), new byte[0]); + KopEventManager.getKopPath(), new byte[0]); ZooKeeperUtils.tryCreatePath(brokerService.pulsar().getZkClient(), - KopZkClient.getDeleteTopicsZNodePath(), new byte[0]); + KopEventManager.getDeleteTopicsPath(), new byte[0]); PulsarAdmin pulsarAdmin; try { @@ -332,12 +328,12 @@ public void start(BrokerService service) { throw new IllegalStateException(e); } - kopZkClient = createKopZkClient(kafkaConfig.getZookeeperServers()); - // init and start group coordinator startGroupCoordinator(pulsarClient); // init KopEventManager - kopEventManager = new KopEventManager(groupCoordinator, kopZkClient, adminManager); + kopEventManager = new KopEventManager(groupCoordinator, + adminManager, + brokerService.getPulsar().getLocalMetadataStore()); kopEventManager.start(); // and listener for Offset topics load/unload @@ -518,15 +514,4 @@ private void loadTxnLogTopics(TransactionCoordinator txnCoordinator) throws Exce return LOOKUP_CLIENT_MAP.computeIfAbsent(pulsarService, ignored -> new LookupClient(pulsarService)); } - private static KopZkClient createKopZkClient(String zkConnect) { - ZooKeeperClient zooKeeperClient = new ZooKeeperClient(zkConnect, - 30000, - 15000, - 10); - - zooKeeperClient.init(); - - return new KopZkClient(zooKeeperClient); - } - } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java index f6a1886748..cd80b98b0b 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java @@ -51,7 +51,6 @@ import io.streamnative.pulsar.handlers.kop.stats.StatsLogger; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; -import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.MessageIdUtils; import io.streamnative.pulsar.handlers.kop.utils.OffsetFinder; import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperUtils; @@ -2031,7 +2030,7 @@ protected void handleDeleteTopics(KafkaHeaderAndRequest deleteTopics, deleteTopicsResponse.forEach((topic, errors) -> { if (errors == Errors.NONE) { ZooKeeperUtils.tryCreatePath(pulsarService.getZkClient(), - KopZkClient.getDeleteTopicsZNodePath() + "/" + topic, + KopEventManager.getDeleteTopicsPath() + "/" + topic, new byte[0]); } }); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index 000daa8f7a..b021679705 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -23,12 +23,11 @@ import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupCoordinator; import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; -import io.streamnative.pulsar.handlers.kop.utils.KopZkClient; import io.streamnative.pulsar.handlers.kop.utils.ShutdownableThread; -import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.locks.ReentrantLock; @@ -38,12 +37,14 @@ import lombok.extern.slf4j.Slf4j; import org.apache.kafka.common.Node; import org.apache.kafka.common.TopicPartition; +import org.apache.pulsar.broker.loadbalance.LoadManager; import org.apache.pulsar.common.util.Murmur3_32Hash; -import org.apache.zookeeper.KeeperException; +import org.apache.pulsar.metadata.api.GetResult; +import org.apache.pulsar.metadata.api.MetadataStore; +import org.apache.pulsar.metadata.api.Notification; @Slf4j public class KopEventManager { - private static final String END_POINT_SEPARATOR = ","; private static final String REGEX = "^(.*)://\\[?([0-9a-zA-Z\\-%._:]*)\\]?:(-?[0-9]+)"; private static final Pattern PATTERN = Pattern.compile(REGEX); @@ -53,24 +54,24 @@ public class KopEventManager { new LinkedBlockingQueue<>(); private final KopEventThread thread = new KopEventThread(kopEventThreadName); - private final GroupCoordinator coordinator; - private final KopZkClient kopZkClient; - private final AdminManager adminManager; + private static GroupCoordinator coordinator; + private static AdminManager adminManager; private final DeletionTopicsHandler deletionTopicsHandler; private final BrokersChangeHandler brokersChangeHandler; + private static MetadataStore metadataStore; public KopEventManager(GroupCoordinator coordinator, - KopZkClient kopZkClient, - AdminManager adminManager) { + AdminManager adminManager, + MetadataStore metadataStore) { this.coordinator = coordinator; - this.kopZkClient = kopZkClient; this.adminManager = adminManager; this.deletionTopicsHandler = new DeletionTopicsHandler(this); this.brokersChangeHandler = new BrokersChangeHandler(this); + this.metadataStore = metadataStore; } public void start() { - registerZNodeChildChangeHandler(); + registerChildChangeHandler(); thread.start(); } @@ -126,37 +127,43 @@ protected void doWork() { } - private void registerZNodeChildChangeHandler() { - kopZkClient.registerZNodeChildChangeHandler(deletionTopicsHandler); - kopZkClient.registerZNodeChildChangeHandler(brokersChangeHandler); - try { - // Really register ZNodeChildChange to zk. - kopZkClient.getTopicDeletions(); - // init local kop brokers cache - getBrokers(kopZkClient.getBrokers()); - } catch (InterruptedException | KeeperException e) { - log.error("registerZNodeChildChangeHandler have failed with an error {}", e.getMessage()); - e.printStackTrace(); + private void registerChildChangeHandler() { + metadataStore.registerListener(this::handleChildChangePathNotification); + + // Really register ChildChange notification. + metadataStore.getChildren(getDeleteTopicsPath()); + // init local kop brokers cache + getBrokers(metadataStore.getChildren(getBrokersChangePath()).join()); + } + + private void handleChildChangePathNotification(Notification notification) { + if (notification.getPath().equals(LoadManager.LOADBALANCE_BROKERS_ROOT)) { + this.brokersChangeHandler.handleChildChange(); + } else if (notification.getPath().equals(getDeleteTopicsPath())) { + this.deletionTopicsHandler.handleChildChange(); } } - private void getBrokers(List pulsarBrokers) { + private static void getBrokers(List pulsarBrokers) { HashSet kopBrokers = Sets.newHashSet(); pulsarBrokers.forEach(broker -> { try { - byte[] brokerInfo = - kopZkClient.getDataForPath(KopZkClient.getBrokersChangeZNodePath() + "/" + broker); - JsonObject jsonObject = parseJsonObject(new String(brokerInfo)); - JsonObject protocols = jsonObject.getAsJsonObject("protocols"); - JsonElement element = protocols.get("kafka"); - - if (element != null) { - String kopBrokerStr = element.getAsString(); - Node kopNode = getNode(kopBrokerStr); - kopBrokers.add(kopNode); + Optional brokerData = metadataStore.get( + getBrokersChangePath() + "/" + broker).join(); + + if (brokerData.isPresent()) { + JsonObject jsonObject = parseJsonObject(new String(brokerData.get().getValue())); + JsonObject protocols = jsonObject.getAsJsonObject("protocols"); + JsonElement element = protocols.get("kafka"); + + if (element != null) { + String kopBrokerStr = element.getAsString(); + Node kopNode = getNode(kopBrokerStr); + kopBrokers.add(kopNode); + } } } catch (Exception e) { - log.error("Get broker {} ZNode data failed which have an error {}", broker, e.getMessage()); + log.error("Get broker {} path data failed which have an error {}", broker, e.getMessage()); e.printStackTrace(); } }); @@ -166,12 +173,12 @@ private void getBrokers(List pulsarBrokers) { adminManager.getBrokers(), oldKopBrokers); } - private JsonObject parseJsonObject(String info) { + private static JsonObject parseJsonObject(String info) { JsonParser parser = new JsonParser(); return parser.parse(info).getAsJsonObject(); } - private Node getNode(String kopBrokerStr) { + private static Node getNode(String kopBrokerStr) { final String errorMessage = "kopBrokerStr " + kopBrokerStr + " is invalid"; final Matcher matcher = PATTERN.matcher(kopBrokerStr); checkState(matcher.find(), errorMessage); @@ -185,49 +192,12 @@ private Node getNode(String kopBrokerStr) { Integer.parseInt(port)); } - class DeletionTopicsHandler implements ZooKeeperClient.ZNodeChildChangeHandler { - private final KopEventManager kopEventManager; - - public DeletionTopicsHandler(KopEventManager kopEventManager) { - this.kopEventManager = kopEventManager; - } - - @Override - public String path() { - return KopZkClient.getDeleteTopicsZNodePath(); - } - - @Override - public void handleChildChange() { - kopEventManager.put(new DeleteTopicsEvent()); - } - } - - class BrokersChangeHandler implements ZooKeeperClient.ZNodeChildChangeHandler { - private final KopEventManager kopEventManager; - - public BrokersChangeHandler(KopEventManager kopEventManager) { - this.kopEventManager = kopEventManager; - } - - @Override - public String path() { - return KopZkClient.getBrokersChangeZNodePath(); - } - - @Override - public void handleChildChange() { - kopEventManager.put(new BrokersChangeEvent()); - } - - } - interface KopEvent { void process(); } - class DeleteTopicsEvent implements KopEvent { + static class DeleteTopicsEvent implements KopEvent { @Override public void process() { @@ -236,7 +206,7 @@ public void process() { } try { - List topicsDeletions = kopZkClient.getTopicDeletions(); + List topicsDeletions = metadataStore.getChildren(getDeleteTopicsPath()).join(); HashSet topicsFullNameDeletionsSets = Sets.newHashSet(); HashSet kopTopicsSet = Sets.newHashSet(); @@ -267,8 +237,10 @@ public void process() { kopTopic -> collectDeleteTopics.contains(kopTopic.getFullName()) ).map(KopTopic::getOriginalName).collect(Collectors.toSet()); - kopZkClient.deleteNodesForPaths( - KopZkClient.getDeleteTopicsZNodePath(), deletedTopics); + deletedTopics.forEach(deletedTopic -> { + metadataStore.delete( + getDeleteTopicsPath() + "/" + deletedTopic, Optional.of((long) -1)); + }); } log.info("GroupMetadata delete topics {}, no matching topics {}", @@ -281,11 +253,11 @@ public void process() { } } - class BrokersChangeEvent implements KopEvent { + static class BrokersChangeEvent implements KopEvent { @Override public void process() { try { - getBrokers(kopZkClient.getBrokers()); + getBrokers(metadataStore.getChildren(getBrokersChangePath()).join()); } catch (Exception e) { log.error("BrokersChangeEvent process have an error {}", e.getMessage()); e.printStackTrace(); @@ -293,4 +265,16 @@ public void process() { } } + public static String getKopPath() { + return "/kop"; + } + + public static String getDeleteTopicsPath() { + return getKopPath() + "/delete_topics"; + } + + public static String getBrokersChangePath() { + return LoadManager.LOADBALANCE_BROKERS_ROOT; + } + } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java deleted file mode 100644 index eb7d99e0a0..0000000000 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/KopZkClient.java +++ /dev/null @@ -1,183 +0,0 @@ -/** - * Licensed 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 io.streamnative.pulsar.handlers.kop.utils; - -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; -import com.google.common.collect.Streams; -import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncRequest; -import io.streamnative.pulsar.handlers.kop.utils.ZooKeeperClient.AsyncResponse; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; -import lombok.extern.slf4j.Slf4j; -import org.apache.pulsar.broker.loadbalance.LoadManager; -import org.apache.zookeeper.KeeperException; - -@Slf4j -public class KopZkClient { - - private final ZooKeeperClient zooKeeperClient; - - public KopZkClient(ZooKeeperClient zooKeeperClient) { - this.zooKeeperClient = zooKeeperClient; - } - - public ZooKeeperClient getZooKeeperClient() { - return zooKeeperClient; - } - - public void registerZNodeChildChangeHandler(ZooKeeperClient.ZNodeChildChangeHandler zNodeChildChangeHandler) { - zooKeeperClient.registerZNodeChildChangeHandler(zNodeChildChangeHandler); - } - - public void unregisterZNodeChildChangeHandler(String path) { - zooKeeperClient.unregisterZNodeChildChangeHandler(path); - } - - private boolean registerZNodeChangeHandlerAndCheckExistence(ZooKeeperClient.ZNodeChangeHandler zNodeChangeHandler) - throws InterruptedException, KeeperException { - zooKeeperClient.registerZNodeChangeHandler(zNodeChangeHandler); - AsyncResponse existsResponse = retryRequestUntilConnected( - new ZooKeeperClient.ExistsRequest(zNodeChangeHandler.path(), Optional.empty())); - switch (existsResponse.getResultCode()) { - case OK: - return true; - case NONODE: - return false; - default: - throw existsResponse.resultException().get(); - } - } - - - private AsyncResponse retryRequestUntilConnected(AsyncRequest request) throws InterruptedException { - return retryRequestsUntilConnected(Sets.newHashSet(request)).get(0); - } - - private List retryRequestsUntilConnected(Set requests) - throws InterruptedException { - Set remainingRequests = requests; - ArrayList responses = Lists.newArrayList(); - HashSet remainingRequestsTmp = Sets.newHashSet(); - while (!remainingRequests.isEmpty()) { - List batchResponses = zooKeeperClient.handleRequests(remainingRequests); - - // Only execute slow path if we find a response with CONNECTIONLOSS - if (batchResponses.stream() - .map(AsyncResponse::getResultCode) - .collect(Collectors.toList()) - .contains(KeeperException.Code.CONNECTIONLOSS)) { - Streams.zip( - remainingRequests.stream(), - batchResponses.stream(), - (request, response) -> { - if (response.getResultCode() == KeeperException.Code.CONNECTIONLOSS) { - remainingRequestsTmp.add(request); - } else { - responses.add(response); - } - return null; - }); - remainingRequests.clear(); - remainingRequests = remainingRequestsTmp; - if (!remainingRequests.isEmpty()) { - zooKeeperClient.waitUntilConnected(); - } - } else { - remainingRequests.clear(); - responses.addAll(batchResponses); - } - } - return responses; - } - - /** - * Get all topics have deleted. - * - * @return list of topics which have deleted. - */ - public List getTopicDeletions() throws InterruptedException, KeeperException { - return getChildren(getDeleteTopicsZNodePath()); - } - - /** - * Get all brokers. - * - * @return list of all pulsar brokers. - */ - public List getBrokers() throws InterruptedException, KeeperException { - return getChildren(getBrokersChangeZNodePath()); - } - - public List getChildren(String path) throws InterruptedException, KeeperException { - ZooKeeperClient.GetChildrenResponse getChildrenResponse = - (ZooKeeperClient.GetChildrenResponse) retryRequestUntilConnected( - new ZooKeeperClient.GetChildrenRequest( - path, - true, - Optional.empty())); - - switch (getChildrenResponse.getResultCode()) { - case OK: - return getChildrenResponse.getChildren(); - case NONODE: - return Collections.emptyList(); - default: - throw getChildrenResponse.resultException().get(); - } - } - - public byte[] getDataForPath(String path) throws InterruptedException, KeeperException { - ZooKeeperClient.GetDataRequest getDataRequest = - new ZooKeeperClient.GetDataRequest(path, Optional.empty()); - ZooKeeperClient.GetDataResponse getDataResponse = - (ZooKeeperClient.GetDataResponse) retryRequestUntilConnected(getDataRequest); - switch (getDataResponse.getResultCode()) { - case OK: - return getDataResponse.getData(); - case NONODE: - return new byte[0]; - default: - throw getDataResponse.resultException().get(); - } - } - - public void deleteNodesForPaths(String prePath, Set paths) throws InterruptedException { - HashSet deleteRequests = Sets.newHashSet(); - paths.forEach(path -> { - deleteRequests.add(new ZooKeeperClient.DeleteRequest( - prePath + "/" + path, -1, Optional.empty())); - }); - - retryRequestsUntilConnected(deleteRequests); - } - - public static String getKopZNodePath() { - return "/kop"; - } - - public static String getDeleteTopicsZNodePath() { - return getKopZNodePath() + "/delete_topics"; - } - - public static String getBrokersChangeZNodePath() { - return LoadManager.LOADBALANCE_BROKERS_ROOT; - } - -} diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java deleted file mode 100644 index e938d99147..0000000000 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/utils/ZooKeeperClient.java +++ /dev/null @@ -1,724 +0,0 @@ -/** - * Licensed 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 io.streamnative.pulsar.handlers.kop.utils; - -import static org.apache.zookeeper.Watcher.Event.EventType.NodeChildrenChanged; -import static org.apache.zookeeper.Watcher.Event.EventType.NodeCreated; -import static org.apache.zookeeper.Watcher.Event.EventType.NodeDataChanged; -import static org.apache.zookeeper.Watcher.Event.EventType.NodeDeleted; - -import com.google.common.collect.Lists; -import java.io.IOException; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.Semaphore; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.ReentrantLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; -import java.util.function.BiConsumer; -import lombok.Getter; -import lombok.extern.slf4j.Slf4j; -import org.apache.zookeeper.CreateMode; -import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.WatchedEvent; -import org.apache.zookeeper.Watcher; -import org.apache.zookeeper.ZooKeeper; -import org.apache.zookeeper.data.ACL; -import org.apache.zookeeper.data.Stat; - - -@Slf4j -@Getter -public class ZooKeeperClient { - private final String connectString; - private final int sessionTimeoutMs; - private final int connectionTimeoutMs; - private int maxInFlightRequests; - private volatile ZooKeeper zooKeeper; - private final ReentrantReadWriteLock initializationLock = new ReentrantReadWriteLock(); - private static final ReentrantLock isConnectedOrExpiredLock = new ReentrantLock(); - private static final Condition isConnectedOrExpiredCondition = isConnectedOrExpiredLock.newCondition(); - private final ConcurrentHashMap zNodeChangeHandlers = - new ConcurrentHashMap<>(); - private final ConcurrentHashMap zNodeChildChangeHandlers = - new ConcurrentHashMap<>(); - private final Semaphore inFlightRequests; - private static final ConcurrentHashMap stateChangeHandlers = - new ConcurrentHashMap<>(); - private static final ScheduledExecutorService expiryScheduler = - new ScheduledThreadPoolExecutor(1); - - public ZooKeeperClient(String connectString, - int sessionTimeoutMs, - int connectionTimeoutMs, - int maxInFlightRequests) { - this.connectString = connectString; - this.sessionTimeoutMs = sessionTimeoutMs; - this.connectionTimeoutMs = connectionTimeoutMs; - this.maxInFlightRequests = maxInFlightRequests; - this.inFlightRequests = new Semaphore(maxInFlightRequests); - } - - public void init() { - log.info("Initializing a new session to {}.", connectString); - try { - this.zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); - waitUntilConnected(connectionTimeoutMs, TimeUnit.MILLISECONDS); - } catch (IOException | InterruptedException e) { - log.error("Initializing a new session failed {}", e.getMessage()); - close(); - } - } - - /** - * Register the handler to ZooKeeperClient. - * This is just a local operation. - * This does not actually register a watcher. - *

- * The watcher is only registered once the user calls handle(AsyncRequest) - * or handle(Seq[AsyncRequest]) with either a GetDataRequest or ExistsRequest. - *

- * NOTE: zookeeper only allows registration to a nonexistent znode with ExistsRequest. - * - * @param zNodeChangeHandler the handler to register - */ - public void registerZNodeChangeHandler(ZNodeChangeHandler zNodeChangeHandler) { - zNodeChangeHandlers.put(zNodeChangeHandler.path(), zNodeChangeHandler); - } - - /** - * Unregister the handler from ZooKeeperClient. This is just a local operation. - * - * @param path the path of the handler to unregister - */ - public void unregisterZNodeChangeHandler(String path) { - zNodeChangeHandlers.remove(path); - } - - /** - * Register the handler to ZooKeeperClient. - * This is just a local operation. - * This does not actually register a watcher. - *

- * The watcher is only registered once the user calls handle(AsyncRequest) - * or handle(Seq[AsyncRequest]) with a GetChildrenRequest. - * - * @param zNodeChildChangeHandler the handler to register - */ - public void registerZNodeChildChangeHandler(ZNodeChildChangeHandler zNodeChildChangeHandler) { - zNodeChildChangeHandlers.put(zNodeChildChangeHandler.path(), zNodeChildChangeHandler); - } - - /** - * Unregister the handler from ZooKeeperClient. This is just a local operation. - * - * @param path the path of the handler to unregister - */ - public void unregisterZNodeChildChangeHandler(String path) { - zNodeChildChangeHandlers.remove(path); - } - - public void registerStateChangeHandler(StateChangeHandler stateChangeHandler) { - try { - initializationLock.readLock().lock(); - if (stateChangeHandler != null) { - stateChangeHandlers.put(stateChangeHandler.name(), stateChangeHandler); - } - } finally { - initializationLock.readLock().unlock(); - } - } - - public void unregisterStateChangeHandler(String name) { - try { - initializationLock.readLock().lock(); - stateChangeHandlers.remove(name); - } finally { - initializationLock.readLock().unlock(); - } - } - - private boolean shouldWatch(AsyncRequest request) { - switch (request.getName()) { - case "GetChildrenRequest": - return zNodeChildChangeHandlers.containsKey(request.getPath()); - case "ExistsRequest": - case "GetDataRequest": - return zNodeChangeHandlers.containsKey(request.getPath()); - default: - throw new IllegalStateException("Unexpected value: " + request.getName()); - } - } - - public void waitUntilConnected() throws InterruptedException { - try { - isConnectedOrExpiredLock.lock(); - waitUntilConnected(Long.MAX_VALUE, TimeUnit.MILLISECONDS); - } finally { - isConnectedOrExpiredLock.unlock(); - } - } - - private void waitUntilConnected(long timeout, TimeUnit timeUnit) throws InterruptedException { - log.info("Waiting until connected."); - long nanos = timeUnit.toNanos(timeout); - try { - isConnectedOrExpiredLock.lock(); - ZooKeeper.States connectionState = zooKeeper.getState(); - while (!connectionState.isConnected() && connectionState.isAlive()) { - if (nanos <= 0) { - throw new ZooKeeperClientTimeoutException( - "Timed out waiting for connection while in state: " + connectionState); - } - nanos = isConnectedOrExpiredCondition.awaitNanos(nanos); - connectionState = zooKeeper.getState(); - } - if (connectionState == ZooKeeper.States.AUTH_FAILED) { - throw new ZooKeeperClientAuthFailedException( - "Auth failed either before or while waiting for connection"); - } else if (connectionState == ZooKeeper.States.CLOSED) { - throw new ZooKeeperClientExpiredException( - "Session expired either before or while waiting for connection"); - } - log.info("Connected."); - } finally { - isConnectedOrExpiredLock.unlock(); - } - } - - public void close() { - log.info("Closing."); - try { - expiryScheduler.shutdown(); - initializationLock.writeLock().lock(); - zNodeChangeHandlers.clear(); - zNodeChildChangeHandlers.clear(); - stateChangeHandlers.clear(); - zooKeeper.close(); - } catch (InterruptedException e) { - log.error("zookeeper close failed {}", e.getMessage()); - } finally { - initializationLock.writeLock().unlock(); - } - log.info("Closed."); - } - - protected List handleRequests(Set requests) - throws InterruptedException { - if (requests.isEmpty()) { - return Lists.newArrayList(); - } else { - CountDownLatch countDownLatch = new CountDownLatch(requests.size()); - ArrayBlockingQueue responseQueue = - new ArrayBlockingQueue<>(requests.size()); - - final BiConsumer responseCallback = (response, throwable) -> { - responseQueue.add(response); - inFlightRequests.release(); - countDownLatch.countDown(); - }; - - for (AsyncRequest request : requests) { - try { - inFlightRequests.acquire(); - initializationLock.readLock().lock(); - send(request, responseCallback); - } catch (Exception e) { - inFlightRequests.release(); - throw e; - } finally { - initializationLock.readLock().unlock(); - } - } - - countDownLatch.await(); - - return Lists.newArrayList(responseQueue.iterator()); - } - } - - private void send(AsyncRequest request, - BiConsumer callback) { - - long sendTimeMs = System.currentTimeMillis(); - switch (request.getName()) { - case "ExistsRequest": - zooKeeper.exists(request.getPath(), shouldWatch(request), - (rc, path, ctx, stat) -> callback.accept(new ExistsResponse( - KeeperException.Code.get(rc), - path, - ctx, - stat, - new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - ), null), request.getCtx()); - break; - case "GetChildrenRequest": - zooKeeper.getChildren(request.getPath(), shouldWatch(request), - (rc, path, ctx, children, stat) -> callback.accept(new GetChildrenResponse( - KeeperException.Code.get(rc), - path, - ctx, - children, - stat, - new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - ), null), request.getCtx()); - break; - case "CreateRequest": - CreateRequest createRequest = (CreateRequest) request; - zooKeeper.create(createRequest.getPath(), - createRequest.getData(), - createRequest.getAcls(), - createRequest.getCreateMode(), - (rc, path, ctx, name, stat) -> callback.accept(new CreateResponse( - KeeperException.Code.get(rc), - path, - ctx, - name, - new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - ), null), createRequest.getCtx()); - break; - case "DeleteRequest": - DeleteRequest deleteRequest = (DeleteRequest) request; - zooKeeper.delete(deleteRequest.getPath(), deleteRequest.getVersion(), - (rc, path, ctx) -> callback.accept(new DeleteResponse( - KeeperException.Code.get(rc), - path, - ctx, - new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - ), null), deleteRequest.getCtx()); - break; - case "GetDataRequest": - zooKeeper.getData(request.getPath(), shouldWatch(request), - (rc, path, ctx, data, stat) -> callback.accept(new GetDataResponse( - KeeperException.Code.get(rc), - path, - ctx, - data, - stat, - new ResponseMetadata(sendTimeMs, System.currentTimeMillis()) - ), null), request.getCtx()); - break; - default: - throw new IllegalStateException("Unexpected value: " + request); - } - - } - - private void scheduleSessionExpiryHandler() { - expiryScheduler.schedule(() -> { - log.info("Session expired."); - reinitialize(); - }, 0, TimeUnit.MILLISECONDS); - } - - private void callBeforeInitializingSession(StateChangeHandler handler) { - try { - handler.beforeInitializingSession(); - } catch (Throwable t) { - log.error("Uncaught error in handler {}, throwable {}", handler.name(), t); - } - } - - private void callAfterInitializingSession(StateChangeHandler handler) { - try { - handler.afterInitializingSession(); - } catch (Throwable t) { - log.error("Uncaught error in handler {}, throwable {}", handler.name(), t); - } - } - - private void reinitialize() { - // Initialization callbacks are invoked outside of the lock to avoid deadlock potential since their completion - // may require additional Zookeeper requests, which will block to acquire the initialization lock - stateChangeHandlers.values().forEach( - this::callBeforeInitializingSession); - - try { - initializationLock.writeLock().lock(); - if (!zooKeeper.getState().isAlive()) { - zooKeeper.close(); - log.info("Initializing a new session to {}.", connectString); - // retry forever until ZooKeeper can be instantiated - boolean connected = false; - while (!connected) { - try { - this.zooKeeper = new ZooKeeper(connectString, sessionTimeoutMs, new ZooKeeperClientWatcher()); - connected = true; - } catch (Exception e) { - log.info("Error when recreating ZooKeeper, retrying after a short sleep", e); - Thread.sleep(1000); - } - } - stateChangeHandlers.values().forEach(this::callAfterInitializingSession); - } - } catch (Exception e) { - log.error("Error before recreating zookeeper when zookeeper close {}", e.getMessage()); - } finally { - initializationLock.writeLock().unlock(); - } - } - - - private class ZooKeeperClientWatcher implements Watcher { - - @Override - public void process(WatchedEvent watchedEvent) { - log.debug("Received event: {}", watchedEvent); - String path = watchedEvent.getPath(); - if (path == null) { - Event.KeeperState state = watchedEvent.getState(); - try { - isConnectedOrExpiredLock.lock(); - isConnectedOrExpiredCondition.signalAll(); - } finally { - isConnectedOrExpiredLock.unlock(); - } - if (state == Event.KeeperState.AuthFailed) { - log.error("Auth failed."); - stateChangeHandlers.values().forEach(StateChangeHandler::onAuthFailure); - } else if (state == Event.KeeperState.Expired) { - scheduleSessionExpiryHandler(); - } - } else { - Event.EventType eventType = watchedEvent.getType(); - - if (eventType == NodeChildrenChanged) { - zNodeChildChangeHandlers.get(path).handleChildChange(); - } else if (eventType == NodeCreated) { - zNodeChangeHandlers.get(path).handleCreation(); - } else if (eventType == NodeDeleted) { - zNodeChangeHandlers.get(path).handleDeletion(); - } else if (eventType == NodeDataChanged) { - zNodeChangeHandlers.get(path).handleDataChange(); - } - } - } - } - - @Getter - abstract static class AsyncRequest { - private final String name; - private final String path; - private final Optional ctx; - - public AsyncRequest(String path, Optional ctx, String name) { - this.path = path; - this.ctx = ctx; - this.name = name; - } - - } - - @Getter - static class ExistsRequest extends AsyncRequest { - private final String name = "ExistsRequest"; - private final String path; - private final Optional ctx; - - public ExistsRequest(String path, Optional ctx) { - super(path, ctx, "ExistsRequest"); - this.path = path; - this.ctx = ctx; - } - } - - @Getter - static class GetChildrenRequest extends AsyncRequest { - private final String name = "GetChildrenRequest"; - private final String path; - private final boolean registerWatch; - private final Optional ctx; - - public GetChildrenRequest(String path, boolean registerWatch, Optional ctx) { - super(path, ctx, "GetChildrenRequest"); - this.path = path; - this.registerWatch = registerWatch; - this.ctx = ctx; - } - } - - @Getter - static class CreateRequest extends AsyncRequest { - private final String name = "CreateRequest"; - private final String path; - private final byte[] data; - private final List acls; - private final CreateMode createMode; - private final Optional ctx; - - CreateRequest(String path, - byte[] data, - List acls, - CreateMode createMode, - Object ctx) { - super(path, Optional.of(createMode), "CreateRequest"); - this.path = path; - this.data = data; - this.acls = acls; - this.createMode = createMode; - this.ctx = Optional.of(ctx); - } - } - - @Getter - static class DeleteRequest extends AsyncRequest { - private final String name = "DeleteRequest"; - private final String path; - private final int version; - private final Optional ctx; - - DeleteRequest(String path, - int version, - Object ctx) { - super(path, Optional.of(ctx), "DeleteRequest"); - this.path = path; - this.version = version; - this.ctx = Optional.of(ctx); - } - } - - @Getter - static class GetDataRequest extends AsyncRequest { - private final String name = "GetDataRequest"; - private final String path; - private final Optional ctx; - - GetDataRequest(String path, Object ctx) { - super(path, Optional.of(ctx), "GetDataRequest"); - this.path = path; - this.ctx = Optional.of(ctx); - } - } - - @Getter - abstract static class AsyncResponse { - private final KeeperException.Code resultCode; - private final String path; - private final Optional ctx; - private final Stat stat; - private final ResponseMetadata metadata; - - protected AsyncResponse(KeeperException.Code resultCode, - String path, - Optional ctx, - Stat stat, - ResponseMetadata metadata) { - this.resultCode = resultCode; - this.path = path; - this.ctx = ctx; - this.stat = stat; - this.metadata = metadata; - } - - public Optional resultException() { - if (resultCode == KeeperException.Code.OK) { - return Optional.empty(); - } - return Optional.of(KeeperException.create(resultCode, path)); - } - - public void maybeThrow() throws KeeperException { - if (resultCode != KeeperException.Code.OK) { - throw KeeperException.create(resultCode, path); - } - } - - } - - @Getter - static class ResponseMetadata { - private final long sendTimeMs; - private final long receivedTimeMs; - - public ResponseMetadata(long sendTimeMs, long receivedTimeMs) { - this.sendTimeMs = sendTimeMs; - this.receivedTimeMs = receivedTimeMs; - } - - private long responseTimeMs() { - return receivedTimeMs - sendTimeMs; - } - } - - - @Getter - static class ExistsResponse extends AsyncResponse { - private final KeeperException.Code resultCode; - private final String path; - private final Optional ctx; - private final Stat stat; - private final ResponseMetadata metadata; - - public ExistsResponse(KeeperException.Code code, - String path, - Object ctx, - Stat stat, - ResponseMetadata metadata) { - super(code, path, Optional.of(ctx), stat, metadata); - this.resultCode = code; - this.path = path; - this.ctx = Optional.of(ctx); - this.stat = stat; - this.metadata = metadata; - } - } - - @Getter - static class GetChildrenResponse extends AsyncResponse { - private final KeeperException.Code resultCode; - private final String path; - private final Optional ctx; - private final List children; - private final Stat stat; - private final ResponseMetadata metadata; - - GetChildrenResponse(KeeperException.Code resultCode, - String path, - Object ctx, - List children, - Stat stat, - ResponseMetadata metadata) { - super(resultCode, path, Optional.of(ctx), stat, metadata); - this.resultCode = resultCode; - this.path = path; - this.ctx = Optional.of(ctx); - this.children = children; - this.stat = stat; - this.metadata = metadata; - } - } - - @Getter - static class CreateResponse extends AsyncResponse { - private final KeeperException.Code resultCode; - private final String path; - private final Optional ctx; - private final String name; - private final ResponseMetadata metadata; - - CreateResponse(KeeperException.Code resultCode, - String path, - Object ctx, - String name, - ResponseMetadata metadata) { - super(resultCode, path, Optional.of(ctx), null, metadata); - this.resultCode = resultCode; - this.path = path; - this.ctx = Optional.of(ctx); - this.name = name; - this.metadata = metadata; - } - } - - @Getter - static class DeleteResponse extends AsyncResponse { - private final KeeperException.Code resultCode; - private final String path; - private final Optional ctx; - private final ResponseMetadata metadata; - - DeleteResponse(KeeperException.Code resultCode, - String path, - Object ctx, - ResponseMetadata metadata) { - super(resultCode, path, Optional.of(ctx), null, metadata); - this.resultCode = resultCode; - this.path = path; - this.ctx = Optional.of(ctx); - this.metadata = metadata; - } - } - - @Getter - static class GetDataResponse extends AsyncResponse { - private final KeeperException.Code resultCode; - private final String path; - private final Optional ctx; - private final byte[] data; - private final Stat stat; - private final ResponseMetadata metadata; - - GetDataResponse(KeeperException.Code resultCode, - String path, - Object ctx, - byte[] data, - Stat stat, - ResponseMetadata metadata) { - super(resultCode, path, Optional.of(ctx), stat, metadata); - this.resultCode = resultCode; - this.path = path; - this.ctx = Optional.of(ctx); - this.data = data; - this.stat = stat; - this.metadata = metadata; - } - } - - public interface StateChangeHandler { - String name(); - - void beforeInitializingSession(); - - void afterInitializingSession(); - - void onAuthFailure(); - } - - public interface ZNodeChangeHandler { - String path(); - - void handleCreation(); - - void handleDeletion(); - - void handleDataChange(); - } - - public interface ZNodeChildChangeHandler { - String path(); - - void handleChildChange(); - } - - static class ZooKeeperClientException extends RuntimeException { - public ZooKeeperClientException(String message) { - super(message); - } - } - - static class ZooKeeperClientExpiredException extends ZooKeeperClientException { - public ZooKeeperClientExpiredException(String message) { - super(message); - } - } - - static class ZooKeeperClientAuthFailedException extends ZooKeeperClientException { - public ZooKeeperClientAuthFailedException(String message) { - super(message); - } - } - - static class ZooKeeperClientTimeoutException extends ZooKeeperClientException { - public ZooKeeperClientTimeoutException(String message) { - super(message); - } - } -} From 18b45ef09a0fa7d74fc67bb75f5369d49de62d83 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 16:55:15 +0800 Subject: [PATCH 06/19] fix spotBugs error --- .../handlers/kop/ChildChangeHandler.java | 4 +-- .../pulsar/handlers/kop/KopEventManager.java | 29 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java index 94bc56dbbf..7543f91c72 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/ChildChangeHandler.java @@ -33,7 +33,7 @@ public String path() { @Override public void handleChildChange() { - kopEventManager.put(new KopEventManager.DeleteTopicsEvent()); + kopEventManager.put(kopEventManager.getDeleteTopicEvent()); } } @@ -51,7 +51,7 @@ public String path() { @Override public void handleChildChange() { - kopEventManager.put(new KopEventManager.BrokersChangeEvent()); + kopEventManager.put(kopEventManager.getBrokersChangeEvent()); } } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index b021679705..de4fdd3f94 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -24,6 +24,7 @@ import io.streamnative.pulsar.handlers.kop.coordinator.group.GroupMetadata; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; import io.streamnative.pulsar.handlers.kop.utils.ShutdownableThread; +import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -54,11 +55,11 @@ public class KopEventManager { new LinkedBlockingQueue<>(); private final KopEventThread thread = new KopEventThread(kopEventThreadName); - private static GroupCoordinator coordinator; - private static AdminManager adminManager; + private final GroupCoordinator coordinator; + private final AdminManager adminManager; private final DeletionTopicsHandler deletionTopicsHandler; private final BrokersChangeHandler brokersChangeHandler; - private static MetadataStore metadataStore; + private final MetadataStore metadataStore; public KopEventManager(GroupCoordinator coordinator, AdminManager adminManager, @@ -108,7 +109,7 @@ public void clearAndPut(KopEvent event) { } } - static class KopEventThread extends ShutdownableThread { + class KopEventThread extends ShutdownableThread { public KopEventThread(String name) { super(name); @@ -144,7 +145,7 @@ private void handleChildChangePathNotification(Notification notification) { } } - private static void getBrokers(List pulsarBrokers) { + private void getBrokers(List pulsarBrokers) { HashSet kopBrokers = Sets.newHashSet(); pulsarBrokers.forEach(broker -> { try { @@ -152,7 +153,7 @@ private static void getBrokers(List pulsarBrokers) { getBrokersChangePath() + "/" + broker).join(); if (brokerData.isPresent()) { - JsonObject jsonObject = parseJsonObject(new String(brokerData.get().getValue())); + JsonObject jsonObject = parseJsonObject(new String(brokerData.get().getValue(), StandardCharsets.UTF_8)); JsonObject protocols = jsonObject.getAsJsonObject("protocols"); JsonElement element = protocols.get("kafka"); @@ -173,12 +174,12 @@ private static void getBrokers(List pulsarBrokers) { adminManager.getBrokers(), oldKopBrokers); } - private static JsonObject parseJsonObject(String info) { + private JsonObject parseJsonObject(String info) { JsonParser parser = new JsonParser(); return parser.parse(info).getAsJsonObject(); } - private static Node getNode(String kopBrokerStr) { + private Node getNode(String kopBrokerStr) { final String errorMessage = "kopBrokerStr " + kopBrokerStr + " is invalid"; final Matcher matcher = PATTERN.matcher(kopBrokerStr); checkState(matcher.find(), errorMessage); @@ -197,7 +198,7 @@ interface KopEvent { void process(); } - static class DeleteTopicsEvent implements KopEvent { + class DeleteTopicsEvent implements KopEvent { @Override public void process() { @@ -253,7 +254,7 @@ public void process() { } } - static class BrokersChangeEvent implements KopEvent { + class BrokersChangeEvent implements KopEvent { @Override public void process() { try { @@ -265,6 +266,14 @@ public void process() { } } + public DeleteTopicsEvent getDeleteTopicEvent() { + return new DeleteTopicsEvent(); + } + + public BrokersChangeEvent getBrokersChangeEvent() { + return new BrokersChangeEvent(); + } + public static String getKopPath() { return "/kop"; } From b373443762285819bf561c4fe346fd77d994a91b Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 17:08:21 +0800 Subject: [PATCH 07/19] fix checkstyle error --- .../streamnative/pulsar/handlers/kop/KopEventManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index de4fdd3f94..222dc7be7d 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -14,7 +14,6 @@ package io.streamnative.pulsar.handlers.kop; import static com.google.common.base.Preconditions.checkState; -import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.Sets; import com.google.gson.JsonElement; @@ -153,7 +152,8 @@ private void getBrokers(List pulsarBrokers) { getBrokersChangePath() + "/" + broker).join(); if (brokerData.isPresent()) { - JsonObject jsonObject = parseJsonObject(new String(brokerData.get().getValue(), StandardCharsets.UTF_8)); + JsonObject jsonObject = parseJsonObject( + new String(brokerData.get().getValue(), StandardCharsets.UTF_8)); JsonObject protocols = jsonObject.getAsJsonObject("protocols"); JsonElement element = protocols.get("kafka"); @@ -188,7 +188,7 @@ private Node getNode(String kopBrokerStr) { String port = matcher.group(3); return new Node( - Murmur3_32Hash.getInstance().makeHash((host + port).getBytes(UTF_8)), + Murmur3_32Hash.getInstance().makeHash((host + port).getBytes(StandardCharsets.UTF_8)), host, Integer.parseInt(port)); } From 190ea290d21b680dbab3db3af0bcd420460972a6 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 17:14:44 +0800 Subject: [PATCH 08/19] fix spotBugs error --- .../io/streamnative/pulsar/handlers/kop/KopEventManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index 222dc7be7d..c5c7ea1ebc 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -108,7 +108,7 @@ public void clearAndPut(KopEvent event) { } } - class KopEventThread extends ShutdownableThread { + static class KopEventThread extends ShutdownableThread { public KopEventThread(String name) { super(name); From 1f5671cdbbaab7132d23068d757a5e5bc81766d6 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 22:05:45 +0800 Subject: [PATCH 09/19] add tests --- .../pulsar/handlers/kop/KopEventManager.java | 4 +- .../handlers/kop/KopEventManagerTest.java | 33 +++ .../handlers/kop/KafkaRequestHandlerTest.java | 30 +++ .../handlers/kop/KopEventManagerTest.java | 195 ++++++++++++++++++ 4 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 kafka-impl/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java create mode 100644 tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index c5c7ea1ebc..06a70cd448 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -15,6 +15,7 @@ import static com.google.common.base.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Sets; import com.google.gson.JsonElement; import com.google.gson.JsonObject; @@ -179,7 +180,8 @@ private JsonObject parseJsonObject(String info) { return parser.parse(info).getAsJsonObject(); } - private Node getNode(String kopBrokerStr) { + @VisibleForTesting + public static Node getNode(String kopBrokerStr) { final String errorMessage = "kopBrokerStr " + kopBrokerStr + " is invalid"; final Matcher matcher = PATTERN.matcher(kopBrokerStr); checkState(matcher.find(), errorMessage); diff --git a/kafka-impl/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/kafka-impl/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java new file mode 100644 index 0000000000..805fce2396 --- /dev/null +++ b/kafka-impl/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -0,0 +1,33 @@ +/** + * Licensed 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 io.streamnative.pulsar.handlers.kop; + +import org.apache.kafka.common.Node; +import org.testng.Assert; +import org.testng.annotations.Test; + +public class KopEventManagerTest { + + @Test + public void testGetNode() { + final String host = "localhost"; + final int port = 9120; + final String securityProtocol = "SASL_PLAINTEXT"; + final String brokerStr = securityProtocol + "://" + host + ":" + port; + Node node = KopEventManager.getNode(brokerStr); + Assert.assertEquals(node.host(), host); + Assert.assertEquals(node.port(), port); + } + +} diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandlerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandlerTest.java index 9b374f0616..a445d92be9 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandlerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandlerTest.java @@ -710,4 +710,34 @@ public void testMetadataForNonPartitionedTopic(short version) throws Exception { assertEquals(response.topicMetadata().size(), 1); assertEquals(response.errors().size(), 0); } + + @Test(timeOut = 10000) + public void testDeleteTopicsAndCheckChildPath() throws Exception { + Properties props = new Properties(); + props.put(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, "localhost:" + getKafkaBrokerPort()); + + @Cleanup + AdminClient kafkaAdmin = AdminClient.create(props); + Map topicToNumPartitions = new HashMap(){{ + put("testCreateTopics-0", 1); + put("testCreateTopics-1", 3); + put("my-tenant/my-ns/testCreateTopics-2", 1); + put("persistent://my-tenant/my-ns/testCreateTopics-3", 5); + }}; + // create + createTopicsByKafkaAdmin(kafkaAdmin, topicToNumPartitions); + verifyTopicsCreatedByPulsarAdmin(topicToNumPartitions); + // delete + deleteTopicsByKafkaAdmin(kafkaAdmin, topicToNumPartitions.keySet()); + verifyTopicsDeletedByPulsarAdmin(topicToNumPartitions); + // check deleted topics path + List deletedTopics = handler.getPulsarService() + .getBrokerService() + .getPulsar() + .getLocalMetadataStore() + .getChildren(KopEventManager.getDeleteTopicsPath()) + .join(); + + assertTrue(topicToNumPartitions.keySet().containsAll(deletedTopics)); + } } diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java new file mode 100644 index 0000000000..c9135a5416 --- /dev/null +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -0,0 +1,195 @@ +/** + * Licensed 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 io.streamnative.pulsar.handlers.kop; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +import com.google.common.collect.Lists; +import java.time.Duration; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.TimeUnit; +import org.apache.kafka.clients.admin.AdminClient; +import org.apache.kafka.clients.admin.AdminClientConfig; +import org.apache.kafka.clients.admin.ConsumerGroupDescription; +import org.apache.kafka.clients.admin.NewTopic; +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.ConsumerRecords; +import org.apache.kafka.clients.consumer.KafkaConsumer; +import org.apache.kafka.clients.producer.KafkaProducer; +import org.apache.kafka.clients.producer.ProducerConfig; +import org.apache.kafka.clients.producer.ProducerRecord; +import org.apache.kafka.common.ConsumerGroupState; +import org.apache.kafka.common.serialization.StringDeserializer; +import org.apache.kafka.common.serialization.StringSerializer; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + + +public class KopEventManagerTest extends KopProtocolHandlerTestBase { + private AdminClient adminClient; + private String broker; + + @BeforeMethod + @Override + protected void setup() throws Exception { + super.internalSetup(); + final EndPoint plainEndPoint = getPlainEndPoint(); + this.broker = plainEndPoint.getHostname() + ":" + plainEndPoint.getPort(); + Properties adminPro = new Properties(); + adminPro.put(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, broker); + this.adminClient = AdminClient.create(adminPro); + } + + @AfterMethod + @Override + protected void cleanup() throws Exception { + adminClient.close(); + super.internalCleanup(); + } + + @Test(invocationCount = 100) + public void testDeleteTopicsAndDescribeGroupStable() throws Exception { + // 1. create topics + final String topic1 = "test-topic1"; + final String topic2 = "test-topic2"; + final String topic3 = "test-topic3"; + List topicsList = Lists.newArrayList(); + NewTopic newTopic1 = new NewTopic(topic1, 1, (short) 1); + topicsList.add(newTopic1); + NewTopic newTopic2 = new NewTopic(topic2, 1, (short) 1); + topicsList.add(newTopic2); + NewTopic newTopic3 = new NewTopic(topic3, 1, (short) 1); + topicsList.add(newTopic3); + + adminClient.createTopics(topicsList).all().get(); + + final String groupId1 = "test-group1"; + final String groupId2 = "test-group2"; + + // 2. send messages + int totalMsg = 15; + final Properties producerPro = new Properties(); + producerPro.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); + producerPro.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); + producerPro.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class); + final KafkaProducer kafkaProducer = new KafkaProducer<>(producerPro); + String topic = topic1; + for (int i = 0; i < totalMsg; i++) { + if (i >= 10) { + topic = topic3; + } else if (i >= 5) { + topic = topic2; + } + + kafkaProducer.send(new ProducerRecord<>(topic, null, "test-value" + i)); + } + kafkaProducer.close(); + + // 3. check group state which only consumed one topic + final Properties properties = new Properties(); + properties.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); + properties.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); + properties.put(ConsumerConfig.GROUP_ID_CONFIG, groupId1); + + final KafkaConsumer kafkaConsumer1 = new KafkaConsumer<>(properties); + kafkaConsumer1.subscribe(Collections.singletonList(topic1)); + int consumeCount = 0; + while (consumeCount < 5) { + ConsumerRecords records = kafkaConsumer1.poll(Duration.ofMillis(5000)); + consumeCount += records.count(); + } + // 4. manually trigger commit offset + kafkaConsumer1.commitSync(Duration.ofMillis(2000)); + // 5. check group state must be Stable + Map describeGroup1 = + adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup1.containsKey(groupId1)); + assertEquals(ConsumerGroupState.STABLE, describeGroup1.get(groupId1).state()); + // 6. close consumer1 + kafkaConsumer1.close(); + // 7. check group state must be Empty + Map describeGroup2 = + adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup1.containsKey(groupId1)); + assertEquals(ConsumerGroupState.EMPTY, describeGroup2.get(groupId1).state()); + // 8. delete topic1 + adminClient.deleteTopics(Collections.singletonList(topic1)); + // 9. describe group who only consume topic1 which have been deleted + Map describeGroup3 = + adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup3.containsKey(groupId1)); + // 10. check group state must be Dead + assertEquals(ConsumerGroupState.DEAD, describeGroup3.get(groupId1).state()); + + // 11. check group state which consumed two topics + properties.put(ConsumerConfig.GROUP_ID_CONFIG, groupId2); + final KafkaConsumer kafkaConsumer2 = new KafkaConsumer<>(properties); + kafkaConsumer2.subscribe(Arrays.asList(topic2, topic3)); + while (consumeCount < 15) { + ConsumerRecords records = kafkaConsumer2.poll(Duration.ofMillis(5000)); + consumeCount += records.count(); + } + // 12. manually trigger commit offset + kafkaConsumer2.commitSync(Duration.ofMillis(2000)); + + // 13. check group state must be Stable + Map describeGroup4 = + adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup4.containsKey(groupId2)); + assertEquals(ConsumerGroupState.STABLE, describeGroup4.get(groupId2).state()); + + // 14. close consumer2 + kafkaConsumer2.close(); + + // 15. check group state must be Empty + Map describeGroup5 = + adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup5.containsKey(groupId2)); + assertEquals(ConsumerGroupState.EMPTY, describeGroup5.get(groupId2).state()); + + // 16. delete topic2 and topic3 + List deleteTopics = Lists.newArrayList(); + deleteTopics.add(topic2); + deleteTopics.add(topic3); + adminClient.deleteTopics(deleteTopics); + + // 17. check group state must be Dead + Map describeGroup6 = + adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup6.containsKey(groupId2)); + assertEquals(ConsumerGroupState.DEAD, describeGroup6.get(groupId2).state()); + + } + +} From 99b1534c429083ae1ccf83b7bfa95ba12954723e Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 22:11:35 +0800 Subject: [PATCH 10/19] finx Codacy Static Code Analysis error --- .../streamnative/pulsar/handlers/kop/KopEventManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index c9135a5416..374fbd3435 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -65,7 +65,7 @@ protected void cleanup() throws Exception { } @Test(invocationCount = 100) - public void testDeleteTopicsAndDescribeGroupStable() throws Exception { + public void testDeleteTopicsAndGroupStable() throws Exception { // 1. create topics final String topic1 = "test-topic1"; final String topic2 = "test-topic2"; From f946b648785e622a82ec8a1d1d76ee7fdb647585 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 23:14:14 +0800 Subject: [PATCH 11/19] fix test failed --- .../pulsar/handlers/kop/KopEventManagerTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index 374fbd3435..35356a7468 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -65,7 +65,7 @@ protected void cleanup() throws Exception { } @Test(invocationCount = 100) - public void testDeleteTopicsAndGroupStable() throws Exception { + public void testDeleteTopicsGroupStable() throws Exception { // 1. create topics final String topic1 = "test-topic1"; final String topic2 = "test-topic2"; @@ -137,6 +137,9 @@ public void testDeleteTopicsAndGroupStable() throws Exception { assertEquals(ConsumerGroupState.EMPTY, describeGroup2.get(groupId1).state()); // 8. delete topic1 adminClient.deleteTopics(Collections.singletonList(topic1)); + // Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore + // and operated asynchronously by the kopEventThread, we will wait here for a while + Thread.sleep(3000); // 9. describe group who only consume topic1 which have been deleted Map describeGroup3 = adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) @@ -181,6 +184,9 @@ public void testDeleteTopicsAndGroupStable() throws Exception { deleteTopics.add(topic2); deleteTopics.add(topic3); adminClient.deleteTopics(deleteTopics); + // Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore + // and operated asynchronously by the kopEventThread, we will wait here for a while + Thread.sleep(3000); // 17. check group state must be Dead Map describeGroup6 = From 36d026e49358a841e35ae5623c82131c48d64c76 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Tue, 7 Sep 2021 23:18:04 +0800 Subject: [PATCH 12/19] fix conflicts with master branch --- .../pulsar/handlers/kop/KafkaChannelInitializer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java index 24d76ef6c0..4d43d3fb45 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaChannelInitializer.java @@ -92,9 +92,9 @@ protected void initChannel(SocketChannel ch) throws Exception { ch.pipeline().addLast("frameDecoder", new LengthFieldBasedFrameDecoder(MAX_FRAME_LENGTH, 0, 4, 0, 4)); ch.pipeline().addLast("handler", - new KafkaRequestHandler(pulsarService, kafkaConfig, - groupCoordinator, transactionCoordinator, adminManager, - localBrokerDataCache, enableTls, advertisedEndPoint, statsLogger)); + new KafkaRequestHandler(pulsarService, kafkaConfig, + groupCoordinator, transactionCoordinator, adminManager, localBrokerDataCache, + enableTls, advertisedEndPoint, statsLogger)); } } From d18bd86cfeb3cfd332d1e9fa633e1c60b26c0fdf Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Wed, 8 Sep 2021 00:00:20 +0800 Subject: [PATCH 13/19] remove invocationCount from testDeleteTopicsGroupStable and fix test failed --- .../handlers/kop/KopEventManagerTest.java | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index 35356a7468..9c9b6a53dd 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -23,7 +23,10 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + import org.apache.kafka.clients.admin.AdminClient; import org.apache.kafka.clients.admin.AdminClientConfig; import org.apache.kafka.clients.admin.ConsumerGroupDescription; @@ -64,7 +67,7 @@ protected void cleanup() throws Exception { super.internalCleanup(); } - @Test(invocationCount = 100) + @Test public void testDeleteTopicsGroupStable() throws Exception { // 1. create topics final String topic1 = "test-topic1"; @@ -137,17 +140,12 @@ public void testDeleteTopicsGroupStable() throws Exception { assertEquals(ConsumerGroupState.EMPTY, describeGroup2.get(groupId1).state()); // 8. delete topic1 adminClient.deleteTopics(Collections.singletonList(topic1)); - // Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore + // 9. Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore // and operated asynchronously by the kopEventThread, we will wait here for a while Thread.sleep(3000); - // 9. describe group who only consume topic1 which have been deleted - Map describeGroup3 = - adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) - .all() - .get(2000, TimeUnit.MILLISECONDS); - assertTrue(describeGroup3.containsKey(groupId1)); - // 10. check group state must be Dead - assertEquals(ConsumerGroupState.DEAD, describeGroup3.get(groupId1).state()); + // 10. describe group who only consume topic1 which have been deleted + // check group state must be Dead + retryUntilTrue(groupId1, 20); // 11. check group state which consumed two topics properties.put(ConsumerConfig.GROUP_ID_CONFIG, groupId2); @@ -189,12 +187,27 @@ public void testDeleteTopicsGroupStable() throws Exception { Thread.sleep(3000); // 17. check group state must be Dead - Map describeGroup6 = - adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) - .all() - .get(2000, TimeUnit.MILLISECONDS); - assertTrue(describeGroup6.containsKey(groupId2)); - assertEquals(ConsumerGroupState.DEAD, describeGroup6.get(groupId2).state()); + retryUntilTrue(groupId2, 20); + + } + + private void retryUntilTrue(String groupId, int timeOutSec) throws Exception { + long startTimeMs = System.currentTimeMillis(); + long deadTimeMs = startTimeMs + timeOutSec * 1000L; + + Map describeGroup = null; + + while (System.currentTimeMillis() < deadTimeMs) { + describeGroup = adminClient.describeConsumerGroups(Collections.singletonList(groupId)) + .all() + .get(2000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup.containsKey(groupId)); + if (describeGroup.get(groupId).state().name().equals("Dead")) { + break; + } + } + + assertEquals(ConsumerGroupState.DEAD, describeGroup.get(groupId).state()); } From 62da2fcd4f75060ad435d053e92a137e875b4119 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Wed, 8 Sep 2021 00:09:50 +0800 Subject: [PATCH 14/19] fix checkstyle error and Codacy Static Code Analysis error --- .../pulsar/handlers/kop/KopEventManagerTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index 9c9b6a53dd..4dd9de76e4 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -23,10 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; - import org.apache.kafka.clients.admin.AdminClient; import org.apache.kafka.clients.admin.AdminClientConfig; import org.apache.kafka.clients.admin.ConsumerGroupDescription; @@ -68,7 +65,7 @@ protected void cleanup() throws Exception { } @Test - public void testDeleteTopicsGroupStable() throws Exception { + public void testGroupStable() throws Exception { // 1. create topics final String topic1 = "test-topic1"; final String topic2 = "test-topic2"; From 819a8fea02a31de1c831bcf447683a622e53bc06 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Wed, 8 Sep 2021 00:23:20 +0800 Subject: [PATCH 15/19] fix Codacy Static Code Analysis error --- .../handlers/kop/KopEventManagerTest.java | 57 ++++++++++--------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index 4dd9de76e4..385929aeef 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -45,6 +45,9 @@ public class KopEventManagerTest extends KopProtocolHandlerTestBase { private AdminClient adminClient; private String broker; + private final String topic1 = "test-topic1"; + private final String topic2 = "test-topic2"; + private final String topic3 = "test-topic3"; @BeforeMethod @Override @@ -65,11 +68,8 @@ protected void cleanup() throws Exception { } @Test - public void testGroupStable() throws Exception { + public void testGroupState() throws Exception { // 1. create topics - final String topic1 = "test-topic1"; - final String topic2 = "test-topic2"; - final String topic3 = "test-topic3"; List topicsList = Lists.newArrayList(); NewTopic newTopic1 = new NewTopic(topic1, 1, (short) 1); topicsList.add(newTopic1); @@ -80,27 +80,8 @@ public void testGroupStable() throws Exception { adminClient.createTopics(topicsList).all().get(); - final String groupId1 = "test-group1"; - final String groupId2 = "test-group2"; - // 2. send messages - int totalMsg = 15; - final Properties producerPro = new Properties(); - producerPro.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); - producerPro.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); - producerPro.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class); - final KafkaProducer kafkaProducer = new KafkaProducer<>(producerPro); - String topic = topic1; - for (int i = 0; i < totalMsg; i++) { - if (i >= 10) { - topic = topic3; - } else if (i >= 5) { - topic = topic2; - } - - kafkaProducer.send(new ProducerRecord<>(topic, null, "test-value" + i)); - } - kafkaProducer.close(); + sendMessages(); // 3. check group state which only consumed one topic final Properties properties = new Properties(); @@ -108,6 +89,7 @@ public void testGroupStable() throws Exception { properties.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); + final String groupId1 = "test-group1"; properties.put(ConsumerConfig.GROUP_ID_CONFIG, groupId1); final KafkaConsumer kafkaConsumer1 = new KafkaConsumer<>(properties); @@ -142,9 +124,10 @@ public void testGroupStable() throws Exception { Thread.sleep(3000); // 10. describe group who only consume topic1 which have been deleted // check group state must be Dead - retryUntilTrue(groupId1, 20); + retryUntilStateDead(groupId1, 20); // 11. check group state which consumed two topics + final String groupId2 = "test-group2"; properties.put(ConsumerConfig.GROUP_ID_CONFIG, groupId2); final KafkaConsumer kafkaConsumer2 = new KafkaConsumer<>(properties); kafkaConsumer2.subscribe(Arrays.asList(topic2, topic3)); @@ -184,11 +167,31 @@ public void testGroupStable() throws Exception { Thread.sleep(3000); // 17. check group state must be Dead - retryUntilTrue(groupId2, 20); + retryUntilStateDead(groupId2, 20); + + } + + private void sendMessages() { + int totalMsg = 15; + final Properties producerPro = new Properties(); + producerPro.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); + producerPro.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); + producerPro.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class); + final KafkaProducer kafkaProducer = new KafkaProducer<>(producerPro); + String topic = topic1; + for (int i = 0; i < totalMsg; i++) { + if (i >= 10) { + topic = topic3; + } else if (i >= 5) { + topic = topic2; + } + kafkaProducer.send(new ProducerRecord<>(topic, null, "test-value" + i)); + } + kafkaProducer.close(); } - private void retryUntilTrue(String groupId, int timeOutSec) throws Exception { + private void retryUntilStateDead(String groupId, int timeOutSec) throws Exception { long startTimeMs = System.currentTimeMillis(); long deadTimeMs = startTimeMs + timeOutSec * 1000L; From 3112e6885d47a6d900d3984c0ef15a456b91fc61 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Wed, 8 Sep 2021 00:41:46 +0800 Subject: [PATCH 16/19] fix Codacy Static Code Analysis error --- .../handlers/kop/KopEventManagerTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index 385929aeef..e3ffe29d8b 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import org.apache.kafka.clients.admin.AdminClient; import org.apache.kafka.clients.admin.AdminClientConfig; @@ -70,19 +71,9 @@ protected void cleanup() throws Exception { @Test public void testGroupState() throws Exception { // 1. create topics - List topicsList = Lists.newArrayList(); - NewTopic newTopic1 = new NewTopic(topic1, 1, (short) 1); - topicsList.add(newTopic1); - NewTopic newTopic2 = new NewTopic(topic2, 1, (short) 1); - topicsList.add(newTopic2); - NewTopic newTopic3 = new NewTopic(topic3, 1, (short) 1); - topicsList.add(newTopic3); - - adminClient.createTopics(topicsList).all().get(); - + createTopics(); // 2. send messages sendMessages(); - // 3. check group state which only consumed one topic final Properties properties = new Properties(); properties.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); @@ -137,7 +128,6 @@ public void testGroupState() throws Exception { } // 12. manually trigger commit offset kafkaConsumer2.commitSync(Duration.ofMillis(2000)); - // 13. check group state must be Stable Map describeGroup4 = adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) @@ -165,10 +155,20 @@ public void testGroupState() throws Exception { // Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore // and operated asynchronously by the kopEventThread, we will wait here for a while Thread.sleep(3000); - // 17. check group state must be Dead retryUntilStateDead(groupId2, 20); + } + private void createTopics() throws ExecutionException, InterruptedException { + List topicsList = Lists.newArrayList(); + NewTopic newTopic1 = new NewTopic(topic1, 1, (short) 1); + topicsList.add(newTopic1); + NewTopic newTopic2 = new NewTopic(topic2, 1, (short) 1); + topicsList.add(newTopic2); + NewTopic newTopic3 = new NewTopic(topic3, 1, (short) 1); + topicsList.add(newTopic3); + + adminClient.createTopics(topicsList).all().get(); } private void sendMessages() { From 36962b511a9e15fd36548dacc173e63e3ab5f85b Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Thu, 9 Sep 2021 15:38:56 +0800 Subject: [PATCH 17/19] addressed reviewer's comments --- .../pulsar/handlers/kop/AdminManager.java | 2 +- .../handlers/kop/KafkaRequestHandler.java | 11 +- .../pulsar/handlers/kop/KopEventManager.java | 103 ++++++++----- .../handlers/kop/KopEventManagerTest.java | 138 +++++++++--------- 4 files changed, 135 insertions(+), 119 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java index dd1c630b68..873cde82fa 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java @@ -234,8 +234,8 @@ public Collection getBrokers() { } public void addBrokers(Set brokers) { + brokersCacheLock.writeLock().lock(); try { - brokersCacheLock.writeLock().lock(); this.brokersCache = brokers; } finally { brokersCacheLock.writeLock().unlock(); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java index 3e2d32ddf8..d86539373b 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KafkaRequestHandler.java @@ -28,7 +28,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.Sets; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; @@ -492,7 +491,7 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, // Command response for all topics List allTopicMetadata = Collections.synchronizedList(Lists.newArrayList()); - Set allNodes = Collections.synchronizedSet(Sets.newHashSet()); + List allNodes = Collections.synchronizedList(Lists.newArrayList()); // Get all kop brokers in local cache allNodes.addAll(adminManager.getBrokers()); @@ -675,10 +674,9 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, if (e != null) { log.warn("[{}] Request {}: Exception fetching metadata, will return null Response", ctx.channel(), metadataHar.getHeader(), e); - allNodes.add(newSelfNode()); MetadataResponse finalResponse = new MetadataResponse( - Lists.newArrayList(allNodes), + allNodes, clusterName, controllerId, Collections.emptyList()); @@ -690,10 +688,9 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, if (topicsNumber == 0) { // no topic partitions added, return now. - allNodes.add(newSelfNode()); MetadataResponse finalResponse = new MetadataResponse( - Lists.newArrayList(allNodes), + allNodes, clusterName, controllerId, allTopicMetadata); @@ -760,7 +757,7 @@ protected void handleTopicMetadataRequest(KafkaHeaderAndRequest metadataHar, // TODO: confirm right value for controller_id MetadataResponse finalResponse = new MetadataResponse( - Lists.newArrayList(allNodes), + allNodes, clusterName, controllerId, allTopicMetadata); diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index 06a70cd448..423e0755c2 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -30,7 +30,10 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -40,7 +43,6 @@ import org.apache.kafka.common.TopicPartition; import org.apache.pulsar.broker.loadbalance.LoadManager; import org.apache.pulsar.common.util.Murmur3_32Hash; -import org.apache.pulsar.metadata.api.GetResult; import org.apache.pulsar.metadata.api.MetadataStore; import org.apache.pulsar.metadata.api.Notification; @@ -88,20 +90,20 @@ public void close() { public void put(KopEvent event) { + putLock.lock(); try { - putLock.lock(); queue.put(event); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - log.error("Error put event {} to coordinator event queue {}", event, e); + log.error("Error put event {} to coordinator event queue", event, e); } finally { putLock.unlock(); } } public void clearAndPut(KopEvent event) { + putLock.lock(); try { - putLock.lock(); queue.clear(); put(event); } finally { @@ -122,7 +124,7 @@ protected void doWork() { event = queue.take(); event.process(); } catch (InterruptedException e) { - log.error("Error processing event {}, {}", event, e); + log.error("Error processing event {}", event, e); } } @@ -146,33 +148,57 @@ private void handleChildChangePathNotification(Notification notification) { } private void getBrokers(List pulsarBrokers) { - HashSet kopBrokers = Sets.newHashSet(); + final Set kopBrokers = Sets.newConcurrentHashSet(); + final AtomicInteger pendingBrokers = new AtomicInteger(pulsarBrokers.size()); + CompletableFuture> kopBrokersFuture = new CompletableFuture<>(); pulsarBrokers.forEach(broker -> { - try { - Optional brokerData = metadataStore.get( - getBrokersChangePath() + "/" + broker).join(); - - if (brokerData.isPresent()) { - JsonObject jsonObject = parseJsonObject( - new String(brokerData.get().getValue(), StandardCharsets.UTF_8)); - JsonObject protocols = jsonObject.getAsJsonObject("protocols"); - JsonElement element = protocols.get("kafka"); - - if (element != null) { - String kopBrokerStr = element.getAsString(); - Node kopNode = getNode(kopBrokerStr); - kopBrokers.add(kopNode); + metadataStore.get(getBrokersChangePath() + "/" + broker).whenComplete( + (brokerData, e) -> { + if (e != null) { + log.error("Get broker {} path data failed which have an error", broker, e); + kopBrokersFuture.completeExceptionally(e); + return; + } + + if (brokerData.isPresent()) { + JsonObject jsonObject = parseJsonObject( + new String(brokerData.get().getValue(), StandardCharsets.UTF_8)); + JsonObject protocols = jsonObject.getAsJsonObject("protocols"); + JsonElement element = protocols.get("kafka"); + + if (element != null) { + String kopBrokerStr = element.getAsString(); + Node kopNode = getNode(kopBrokerStr); + kopBrokers.add(kopNode); + } else { + if (log.isDebugEnabled()) { + log.debug("Get broker {} path currently not a kop broker, skip it.", broker); + } + } + } else { + if (log.isDebugEnabled()) { + log.debug("Get broker {} path data empty.", broker); + } + } + + if (pendingBrokers.decrementAndGet() == 0) { + kopBrokersFuture.complete(kopBrokers); + } } - } - } catch (Exception e) { - log.error("Get broker {} path data failed which have an error {}", broker, e.getMessage()); - e.printStackTrace(); + ); + }); + + kopBrokersFuture.whenComplete((brokers, e) -> { + if (e != null) { + log.error("Get pulsar brokers {} failed", pulsarBrokers, e); + return; } + + Collection oldKopBrokers = adminManager.getBrokers(); + adminManager.addBrokers(brokers); + log.info("Refresh kop brokers new cache {}, old brokers cache {}", + adminManager.getBrokers(), oldKopBrokers); }); - Collection oldKopBrokers = adminManager.getBrokers(); - adminManager.addBrokers(kopBrokers); - log.info("Refresh kop brokers new cache {}, old brokers cache {}", - adminManager.getBrokers(), oldKopBrokers); } private JsonObject parseJsonObject(String info) { @@ -209,7 +235,7 @@ public void process() { } try { - List topicsDeletions = metadataStore.getChildren(getDeleteTopicsPath()).join(); + List topicsDeletions = metadataStore.getChildren(getDeleteTopicsPath()).get(); HashSet topicsFullNameDeletionsSets = Sets.newHashSet(); HashSet kopTopicsSet = Sets.newHashSet(); @@ -249,9 +275,8 @@ public void process() { log.info("GroupMetadata delete topics {}, no matching topics {}", deletedTopics, Sets.difference(topicsFullNameDeletionsSets, deletedTopics)); - } catch (Exception e) { - log.error("DeleteTopicsEvent process have an error {}", e.getMessage()); - e.printStackTrace(); + } catch (ExecutionException | InterruptedException e) { + log.error("DeleteTopicsEvent process have an error", e); } } } @@ -259,12 +284,14 @@ public void process() { class BrokersChangeEvent implements KopEvent { @Override public void process() { - try { - getBrokers(metadataStore.getChildren(getBrokersChangePath()).join()); - } catch (Exception e) { - log.error("BrokersChangeEvent process have an error {}", e.getMessage()); - e.printStackTrace(); - } + metadataStore.getChildren(getBrokersChangePath()).whenComplete( + (brokers, e) -> { + if (e != null) { + log.error("BrokersChangeEvent process have an error", e); + return; + } + getBrokers(brokers); + }); } } diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java index e3ffe29d8b..1f94b197c7 100644 --- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java +++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopEventManagerTest.java @@ -42,9 +42,9 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; - public class KopEventManagerTest extends KopProtocolHandlerTestBase { private AdminClient adminClient; + private KafkaProducer kafkaProducer; private String broker; private final String topic1 = "test-topic1"; private final String topic2 = "test-topic2"; @@ -59,21 +59,27 @@ protected void setup() throws Exception { Properties adminPro = new Properties(); adminPro.put(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, broker); this.adminClient = AdminClient.create(adminPro); + final Properties producerPro = new Properties(); + producerPro.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); + producerPro.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); + producerPro.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class); + this.kafkaProducer = new KafkaProducer<>(producerPro); } @AfterMethod @Override protected void cleanup() throws Exception { adminClient.close(); + kafkaProducer.close(); super.internalCleanup(); } - @Test - public void testGroupState() throws Exception { + @Test(timeOut = 6000) + public void testOneTopicGroupState() throws Exception { // 1. create topics - createTopics(); + createTopics(Collections.singletonList(topic1)); // 2. send messages - sendMessages(); + sendOneMessages(topic1); // 3. check group state which only consumed one topic final Properties properties = new Properties(); properties.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); @@ -85,110 +91,96 @@ public void testGroupState() throws Exception { final KafkaConsumer kafkaConsumer1 = new KafkaConsumer<>(properties); kafkaConsumer1.subscribe(Collections.singletonList(topic1)); - int consumeCount = 0; - while (consumeCount < 5) { - ConsumerRecords records = kafkaConsumer1.poll(Duration.ofMillis(5000)); - consumeCount += records.count(); - } - // 4. manually trigger commit offset - kafkaConsumer1.commitSync(Duration.ofMillis(2000)); - // 5. check group state must be Stable + ConsumerRecords records = kafkaConsumer1.poll(Duration.ofMillis(500)); + assertEquals(records.count(), 1); + + // 4. check group state must be Stable Map describeGroup1 = adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) .all() - .get(2000, TimeUnit.MILLISECONDS); + .get(1000, TimeUnit.MILLISECONDS); assertTrue(describeGroup1.containsKey(groupId1)); assertEquals(ConsumerGroupState.STABLE, describeGroup1.get(groupId1).state()); - // 6. close consumer1 + // 5. close consumer1 kafkaConsumer1.close(); - // 7. check group state must be Empty + // 6. check group state must be Empty Map describeGroup2 = adminClient.describeConsumerGroups(Collections.singletonList(groupId1)) .all() - .get(2000, TimeUnit.MILLISECONDS); + .get(1000, TimeUnit.MILLISECONDS); assertTrue(describeGroup1.containsKey(groupId1)); assertEquals(ConsumerGroupState.EMPTY, describeGroup2.get(groupId1).state()); - // 8. delete topic1 + // 7. delete topic1 adminClient.deleteTopics(Collections.singletonList(topic1)); - // 9. Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore - // and operated asynchronously by the kopEventThread, we will wait here for a while - Thread.sleep(3000); - // 10. describe group who only consume topic1 which have been deleted + // 8. describe group who only consume topic1 which have been deleted // check group state must be Dead - retryUntilStateDead(groupId1, 20); + retryUntilStateDead(groupId1, 5); + } + + @Test(timeOut = 6000) + public void testTwoTopicsGroupState() throws Exception { + // 1. create topics + createTopics(Arrays.asList(topic2, topic3)); + // 2. send messages + sendOneMessages(topic2); + sendOneMessages(topic3); - // 11. check group state which consumed two topics + // 3. check group state which consumed two topics + final Properties properties = new Properties(); + properties.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); + properties.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + properties.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); final String groupId2 = "test-group2"; properties.put(ConsumerConfig.GROUP_ID_CONFIG, groupId2); final KafkaConsumer kafkaConsumer2 = new KafkaConsumer<>(properties); kafkaConsumer2.subscribe(Arrays.asList(topic2, topic3)); - while (consumeCount < 15) { - ConsumerRecords records = kafkaConsumer2.poll(Duration.ofMillis(5000)); + int consumeCount = 0; + while (consumeCount < 2) { + ConsumerRecords records = kafkaConsumer2.poll(Duration.ofMillis(500)); consumeCount += records.count(); } - // 12. manually trigger commit offset - kafkaConsumer2.commitSync(Duration.ofMillis(2000)); - // 13. check group state must be Stable - Map describeGroup4 = + // 4. check group state must be Stable + Map describeGroup1 = adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) .all() - .get(2000, TimeUnit.MILLISECONDS); - assertTrue(describeGroup4.containsKey(groupId2)); - assertEquals(ConsumerGroupState.STABLE, describeGroup4.get(groupId2).state()); + .get(1000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup1.containsKey(groupId2)); + assertEquals(ConsumerGroupState.STABLE, describeGroup1.get(groupId2).state()); - // 14. close consumer2 + // 5. close consumer2 kafkaConsumer2.close(); - // 15. check group state must be Empty - Map describeGroup5 = + // 6. check group state must be Empty + Map describeGroup2 = adminClient.describeConsumerGroups(Collections.singletonList(groupId2)) .all() - .get(2000, TimeUnit.MILLISECONDS); - assertTrue(describeGroup5.containsKey(groupId2)); - assertEquals(ConsumerGroupState.EMPTY, describeGroup5.get(groupId2).state()); + .get(1000, TimeUnit.MILLISECONDS); + assertTrue(describeGroup2.containsKey(groupId2)); + assertEquals(ConsumerGroupState.EMPTY, describeGroup2.get(groupId2).state()); - // 16. delete topic2 and topic3 + // 7. delete topic2 and topic3 List deleteTopics = Lists.newArrayList(); deleteTopics.add(topic2); deleteTopics.add(topic3); adminClient.deleteTopics(deleteTopics); - // Since the removal of the deleted partition by the consumer group is triggered by the MetadataStore - // and operated asynchronously by the kopEventThread, we will wait here for a while - Thread.sleep(3000); - // 17. check group state must be Dead - retryUntilStateDead(groupId2, 20); + // 8. check group state must be Dead + retryUntilStateDead(groupId2, 5); } - private void createTopics() throws ExecutionException, InterruptedException { + private void createTopics(List topics) throws ExecutionException, InterruptedException { List topicsList = Lists.newArrayList(); - NewTopic newTopic1 = new NewTopic(topic1, 1, (short) 1); - topicsList.add(newTopic1); - NewTopic newTopic2 = new NewTopic(topic2, 1, (short) 1); - topicsList.add(newTopic2); - NewTopic newTopic3 = new NewTopic(topic3, 1, (short) 1); - topicsList.add(newTopic3); - + topics.forEach( + topic -> { + NewTopic newTopic = new NewTopic(topic, 1, (short) 1); + topicsList.add(newTopic); + } + ); adminClient.createTopics(topicsList).all().get(); } - private void sendMessages() { - int totalMsg = 15; - final Properties producerPro = new Properties(); - producerPro.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, broker); - producerPro.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); - producerPro.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class); - final KafkaProducer kafkaProducer = new KafkaProducer<>(producerPro); - String topic = topic1; - for (int i = 0; i < totalMsg; i++) { - if (i >= 10) { - topic = topic3; - } else if (i >= 5) { - topic = topic2; - } - - kafkaProducer.send(new ProducerRecord<>(topic, null, "test-value" + i)); - } - kafkaProducer.close(); + private void sendOneMessages(String topic) { + kafkaProducer.send(new ProducerRecord<>(topic, null, "test-value")); } private void retryUntilStateDead(String groupId, int timeOutSec) throws Exception { @@ -200,9 +192,9 @@ private void retryUntilStateDead(String groupId, int timeOutSec) throws Exceptio while (System.currentTimeMillis() < deadTimeMs) { describeGroup = adminClient.describeConsumerGroups(Collections.singletonList(groupId)) .all() - .get(2000, TimeUnit.MILLISECONDS); + .get(1000, TimeUnit.MILLISECONDS); assertTrue(describeGroup.containsKey(groupId)); - if (describeGroup.get(groupId).state().name().equals("Dead")) { + if (describeGroup.get(groupId).state().name().equals("DEAD")) { break; } } From e15cd93db3e26a719d6f5cbea57ffd6de86b36ed Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Sun, 12 Sep 2021 15:01:25 +0800 Subject: [PATCH 18/19] addressed reviewer's comments --- .../pulsar/handlers/kop/AdminManager.java | 4 ++-- .../pulsar/handlers/kop/KopEventManager.java | 20 +++++-------------- .../kop/coordinator/group/GroupMetadata.java | 14 ++++++++----- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java index 873cde82fa..8d58ae80fb 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/AdminManager.java @@ -233,10 +233,10 @@ public Collection getBrokers() { return brokersCache; } - public void addBrokers(Set brokers) { + public void setBrokers(Set newBrokers) { brokersCacheLock.writeLock().lock(); try { - this.brokersCache = brokers; + this.brokersCache = newBrokers; } finally { brokersCacheLock.writeLock().unlock(); } diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java index 423e0755c2..8a384015ab 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/KopEventManager.java @@ -30,7 +30,6 @@ import java.util.List; import java.util.Optional; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.atomic.AtomicInteger; @@ -150,13 +149,12 @@ private void handleChildChangePathNotification(Notification notification) { private void getBrokers(List pulsarBrokers) { final Set kopBrokers = Sets.newConcurrentHashSet(); final AtomicInteger pendingBrokers = new AtomicInteger(pulsarBrokers.size()); - CompletableFuture> kopBrokersFuture = new CompletableFuture<>(); + pulsarBrokers.forEach(broker -> { metadataStore.get(getBrokersChangePath() + "/" + broker).whenComplete( (brokerData, e) -> { if (e != null) { log.error("Get broker {} path data failed which have an error", broker, e); - kopBrokersFuture.completeExceptionally(e); return; } @@ -182,23 +180,15 @@ private void getBrokers(List pulsarBrokers) { } if (pendingBrokers.decrementAndGet() == 0) { - kopBrokersFuture.complete(kopBrokers); + Collection oldKopBrokers = adminManager.getBrokers(); + adminManager.setBrokers(kopBrokers); + log.info("Refresh kop brokers new cache {}, old brokers cache {}", + adminManager.getBrokers(), oldKopBrokers); } } ); }); - kopBrokersFuture.whenComplete((brokers, e) -> { - if (e != null) { - log.error("Get pulsar brokers {} failed", pulsarBrokers, e); - return; - } - - Collection oldKopBrokers = adminManager.getBrokers(); - adminManager.addBrokers(brokers); - log.info("Refresh kop brokers new cache {}, old brokers cache {}", - adminManager.getBrokers(), oldKopBrokers); - }); } private JsonObject parseJsonObject(String info) { diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java index 97706fee35..da80c76f71 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java @@ -21,12 +21,14 @@ import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects.ToStringHelper; import com.google.common.base.Supplier; +import com.google.common.collect.Lists; import com.google.common.collect.Sets; import io.streamnative.pulsar.handlers.kop.coordinator.group.MemberMetadata.MemberSummary; import io.streamnative.pulsar.handlers.kop.exceptions.KoPTopicException; import io.streamnative.pulsar.handlers.kop.offset.OffsetAndMetadata; import io.streamnative.pulsar.handlers.kop.utils.CoreUtils; import io.streamnative.pulsar.handlers.kop.utils.KopTopic; +import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -605,24 +607,26 @@ public Map removeOffsets(Stream collectPartitionsWithTopics(Set topics) { - HashSet topicPartitions = Sets.newHashSet(); + public List collectPartitionsWithTopics(Set topics) { + ArrayList topicPartitions = Lists.newArrayList(); topicPartitions.addAll(pendingOffsetCommits.keySet().stream().filter( topicPartition -> topics.contains(topicPartition.topic()) - ).collect(Collectors.toSet())); + && !topicPartitions.contains(topicPartition) + ).collect(Collectors.toList())); pendingTransactionalOffsetCommits.values().stream().map(Map::keySet) .collect(Collectors.toList()).forEach(partitionSet -> { topicPartitions.addAll(partitionSet.stream().filter( - topicPartition -> topics.contains(topicPartition.topic())) + topicPartition -> topics.contains(topicPartition.topic()) + && !topicPartitions.contains(topicPartition)) .collect(Collectors.toList())); }); topicPartitions.addAll(offsets.keySet().stream().filter( topicPartition -> topics.contains(topicPartition.topic()) + && !topicPartitions.contains(topicPartition) ).collect(Collectors.toList())); - return topicPartitions; } From cea832c9550240e2f8377b4d07d7efb9086ce536 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Sun, 12 Sep 2021 15:10:06 +0800 Subject: [PATCH 19/19] addressed reviewer's comments --- .../pulsar/handlers/kop/coordinator/group/GroupMetadata.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java index da80c76f71..a2b478e24b 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/coordinator/group/GroupMetadata.java @@ -612,20 +612,17 @@ public List collectPartitionsWithTopics(Set topics) { topicPartitions.addAll(pendingOffsetCommits.keySet().stream().filter( topicPartition -> topics.contains(topicPartition.topic()) - && !topicPartitions.contains(topicPartition) ).collect(Collectors.toList())); pendingTransactionalOffsetCommits.values().stream().map(Map::keySet) .collect(Collectors.toList()).forEach(partitionSet -> { topicPartitions.addAll(partitionSet.stream().filter( - topicPartition -> topics.contains(topicPartition.topic()) - && !topicPartitions.contains(topicPartition)) + topicPartition -> topics.contains(topicPartition.topic())) .collect(Collectors.toList())); }); topicPartitions.addAll(offsets.keySet().stream().filter( topicPartition -> topics.contains(topicPartition.topic()) - && !topicPartitions.contains(topicPartition) ).collect(Collectors.toList())); return topicPartitions; }