[GitHub] rdhabalia commented on a change in pull request #717: Reader API for C++ client

[gi...@git.apache.org]

rdhabalia commented on a change in pull request #717: Reader API for C++ client
URL: https://github.com/apache/incubator-pulsar/pull/717#discussion_r135599264
 
 

 ##########
 File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
 ##########
 @@ -466,12 +467,22 @@ public void openCursorFailed(ManagedLedgerException exception, Object ctx) {
     private CompletableFuture<? extends Subscription> getNonDurableSubscription(String subscriptionName, MessageId startMessageId) {
         CompletableFuture<Subscription> subscriptionFuture = new CompletableFuture<>();
 
+        // Create a new non-durable cursor only for the first consumer that connects
         Subscription subscription = subscriptions.computeIfAbsent(subscriptionName, name -> {
-            // Create a new non-durable cursor only for the first consumer that connects
             MessageIdImpl msgId = startMessageId != null ? (MessageIdImpl) startMessageId
                     : (MessageIdImpl) MessageId.latest;
 
-            Position startPosition = new PositionImpl(msgId.getLedgerId(), msgId.getEntryId());
+            long ledgerId = msgId.getLedgerId();
+            long entryId = msgId.getEntryId();
+            if (msgId instanceof BatchMessageIdImpl) {
 
 Review comment:
   I think broker is batch-msg agnostic as it treats it as a normal msg (client manages batch-msg business) and it would be better to avoid introducing `BatchMessageIdImpl` at broker-side. Can't we always assign `entryId= entryId-1` while creating  `NonDurableSubscription` ?
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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