[GitHub] ivankelly commented on a change in pull request #1231: Read from compacted topic ledger if available and enabled

[GitBox <gi...@apache.org>]

ivankelly commented on a change in pull request #1231: Read from compacted topic ledger if available and enabled
URL: https://github.com/apache/incubator-pulsar/pull/1231#discussion_r169023061
 
 

 ##########
 File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
 ##########
 @@ -297,7 +297,11 @@ protected void readMoreEntries(Consumer consumer) {
                 log.debug("[{}-{}] Schedule read of {} messages", name, consumer, messagesToRead);
             }
             havePendingRead = true;
-            cursor.asyncReadEntriesOrWait(messagesToRead, this, consumer);
+            if (consumer.readCompacted()) {
+                topic.compactedTopic.asyncReadEntriesOrWait(cursor, messagesToRead, this, consumer);
 
 Review comment:
   it does. This is why we pass the cursor in. Otherwise we'd have something like
   ```java
   if (consumer.readCompacted()) {
       if (topic.compactedTopic.shouldReadFromCompacted(cursor)) {
            topic.compactedTopic.asyncReadEntriesOrWait(cursor, messagesToRead, this, consumer);
       } else {
           // read from cursor as normal
       }
   }
   ```
   However, there could be another thread acting on compactedTopic, so the result from shouldReadFromCompacted may not be consistent with what happens when you try to read. i.e. a compaction could happen in between, and shouldReadFromCompacted could return false, even though in reality compacted data is available. This wouldn't be a huge issue, it just means more data is read. 
   
   

----------------------------------------------------------------
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