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 2021/06/30 01:39:39 UTC

[GitHub] [pulsar] codelipenghui opened a new pull request #11159: Fix reading compacted data with the earliest start position by using Reader API

codelipenghui opened a new pull request #11159:
URL: https://github.com/apache/pulsar/pull/11159


   Currently, if the data has been compacted and the reader tries to read from the start of the topic,
   the read not able to read any data due to the start position will be changed when recovery the NonDurableCursor.
   The start position will be changed to the first position of the ledgers, but there are some earlier data in the
   compacted ledger. So the reader will lose the compacted data.
   
   The fix is adding a force start position option for creating a NonDurableCursor.
   
   1. If there is compacted context, use the first entry of the compacted ledger as the start position
   2. Force seeks to a position without comparing with the mark deletion position / LAC when reading data from the compacted ledger.
   3. Don't rewind the non-durable cursor during the message reading
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API: (no)
     - The schema: (no)
     - The default values of configurations: (no)
     - The wire protocol: (no)
     - The rest endpoints: (no)
     - The admin cli options: (no)
     - Anything that affects deployment: (no)
   
   ### Documentation
   
     - Does this pull request introduce a new feature? (no)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] BewareMyPower commented on a change in pull request #11159: Fix reading compacted data with the earliest start position by using Reader API

Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on a change in pull request #11159:
URL: https://github.com/apache/pulsar/pull/11159#discussion_r661083640



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -896,25 +897,60 @@ public void openCursorFailed(ManagedLedgerException exception, Object ctx) {
                     // The client will then be able to discard the first messages if needed.
                     entryId = msgId.getEntryId() - 1;
                 }
-
-                Position startPosition = new PositionImpl(ledgerId, entryId);
-                ManagedCursor cursor = null;
-                try {
-                    cursor = ledger.newNonDurableCursor(startPosition, subscriptionName, initialPosition);
-                } catch (ManagedLedgerException e) {
-                    return FutureUtil.failedFuture(e);
+                if (ledgerId == -1 && entryId == -1 && readCompacted) {
+                    CompactedTopicImpl compactedTopicImpl = (CompactedTopicImpl) this.compactedTopic;
+                    Optional<CompactedTopicImpl.CompactedTopicContext> optionalCompactedTopicContext;
+                    try {
+                        optionalCompactedTopicContext = compactedTopicImpl.getCompactedTopicContext();
+                    } catch (ExecutionException e) {
+                        subscriptionFuture.completeExceptionally(e.getCause());
+                        return subscriptionFuture;
+                    } catch (InterruptedException e) {
+                        subscriptionFuture.completeExceptionally(e);
+                        Thread.currentThread().interrupt();
+                        return subscriptionFuture;
+                    }

Review comment:
       I think it's not proper to use `getCompactedTopicContext`, which calls `CompletableFuture#get` to wait the future. It's easy to cause deadlock.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Anonymitaet commented on pull request #11159: [WIP] Fix reading compacted data with the earliest start position by using Reader API

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11159:
URL: https://github.com/apache/pulsar/pull/11159#issuecomment-872005773


   @codelipenghui does this affect docs?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #11159: [WIP] Fix reading compacted data with the earliest start position by using Reader API

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #11159:
URL: https://github.com/apache/pulsar/pull/11159#issuecomment-893179765


   https://github.com/apache/pulsar/pull/11287 has fixed the issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #11159: [WIP] Fix reading compacted data with the earliest start position by using Reader API

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #11159:
URL: https://github.com/apache/pulsar/pull/11159#issuecomment-893179765


   https://github.com/apache/pulsar/pull/11287 has fixed the issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui closed pull request #11159: [WIP] Fix reading compacted data with the earliest start position by using Reader API

Posted by GitBox <gi...@apache.org>.
codelipenghui closed pull request #11159:
URL: https://github.com/apache/pulsar/pull/11159


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org