You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/02/24 08:38:00 UTC

[GitHub] [pulsar] avimas opened a new pull request #6407: Java client MultiTopics discovery topic discovery task scheduled twice instead of pendingBatchRecei…

avimas opened a new pull request #6407: Java client MultiTopics discovery  topic discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407
 
 
   
   I wanted to use the topicPattern feature and found out it is broken due to the discovery task being called twice at the same time for the same topic.
   
   I realized that it is called twice because  ConsumerBase class adds itself to the timer upon initialization and MultiTopicsConsumerImpl is doing the same. 
   
   I changed ConsumerBase class to add a specific task without using this.
   
   This change is a trivial
   I tested the change in my environment and verified the issue was fixed.
   I don't know how to add integration tests yet
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] tuteng commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
tuteng commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407#issuecomment-600033442
 
 
   Please rebase master for test @avimas 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] sijie merged pull request #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] avimas commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
avimas commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407#issuecomment-591306119
 
 
   The failed test is not related to the removal of wildcard imports.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] jiazhai commented on a change in pull request #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
jiazhai commented on a change in pull request #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407#discussion_r383703638
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerBase.java
 ##########
 @@ -18,31 +18,9 @@
  */
 package org.apache.pulsar.client.impl;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 import com.google.common.collect.Queues;
-import java.util.Collections;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ConcurrentLinkedQueue;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLongFieldUpdater;
-
 import io.netty.util.Timeout;
-import io.netty.util.TimerTask;
-import org.apache.pulsar.client.api.BatchReceivePolicy;
-import org.apache.pulsar.client.api.Consumer;
-import org.apache.pulsar.client.api.ConsumerEventListener;
-import org.apache.pulsar.client.api.Message;
-import org.apache.pulsar.client.api.MessageId;
-import org.apache.pulsar.client.api.MessageListener;
-import org.apache.pulsar.client.api.Messages;
-import org.apache.pulsar.client.api.PulsarClientException;
-import org.apache.pulsar.client.api.Schema;
-import org.apache.pulsar.client.api.SubscriptionType;
+import org.apache.pulsar.client.api.*;
 
 Review comment:
   Please not use * in the import.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] jiazhai commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
jiazhai commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407#issuecomment-590731397
 
 
   overall lgtm. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] avimas commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
avimas commented on issue #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407#issuecomment-600079479
 
 
   rebase done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] jiazhai commented on a change in pull request #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…

Posted by GitBox <gi...@apache.org>.
jiazhai commented on a change in pull request #6407: [Java client] MultiTopics discovery is broken due to discovery task scheduled twice instead of pendingBatchRecei…
URL: https://github.com/apache/pulsar/pull/6407#discussion_r383704326
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerBase.java
 ##########
 @@ -52,7 +30,15 @@
 import org.apache.pulsar.common.util.FutureUtil;
 import org.apache.pulsar.common.util.collections.GrowableArrayBlockingQueue;
 
-public abstract class ConsumerBase<T> extends HandlerState implements TimerTask, Consumer<T> {
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.*;
 
 Review comment:
   also here. please not use *

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services