You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by si...@apache.org on 2018/10/04 01:30:19 UTC

[pulsar] branch master updated: Fix invalid range error when getting number of entries (#2709)

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new 12962d0  Fix invalid range error when getting number of entries (#2709)
12962d0 is described below

commit 12962d0fecae73f71e1ddfeba4d6bbe65232bcca
Author: massakam <ma...@yahoo-corp.jp>
AuthorDate: Thu Oct 4 10:30:07 2018 +0900

    Fix invalid range error when getting number of entries (#2709)
    
    For a topic with metadata similar to https://github.com/apache/pulsar/pull/2673, IllegalArgumentException may occur in the following line:
    https://github.com/apache/pulsar/blob/b2484d92d5068d4f0699eb9c3d31640cb48f9dd0/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L655
    
    This is the broker log when the exception has occurred:
    [invalid_range_error.txt](https://github.com/apache/pulsar/files/2442924/invalid_range_error.txt)
    
    It is because `readPosition` is ahead of `ledger.getLastPosition().getNext()`, so `ManagedCursorImpl#getNumberOfEntries()` should return 0 as the number of entries to read in that case.
    
    I think this issue and https://github.com/apache/pulsar/pull/2673 are the result of that `ledger.getLastPosition()` is no longer the real last of the managed ledger because of https://github.com/apache/pulsar/pull/1550.
---
 .../org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java  | 10 +++++++++-
 .../org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java  |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
index 64efa85..57c4e62 100644
--- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
+++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
@@ -652,7 +652,15 @@ public class ManagedCursorImpl implements ManagedCursor {
 
     @Override
     public long getNumberOfEntries() {
-        return getNumberOfEntries(Range.closedOpen(readPosition, ledger.getLastPosition().getNext()));
+        if (readPosition.compareTo(ledger.getLastPosition().getNext()) > 0) {
+            if (log.isDebugEnabled()) {
+                log.debug("[{}] [{}] Read position {} is ahead of last position {}. There are no entries to read",
+                        ledger.getName(), name, readPosition, ledger.getLastPosition());
+            }
+            return 0;
+        } else {
+            return getNumberOfEntries(Range.closedOpen(readPosition, ledger.getLastPosition().getNext()));
+        }
     }
 
     @Override
diff --git a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java
index 77d6c6c..d8c48de 100644
--- a/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java
+++ b/managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java
@@ -2759,6 +2759,7 @@ public class ManagedCursorTest extends MockedBookKeeperTestCase {
             public void operationComplete() {
                 assertEquals(cursor.getMarkDeletedPosition(), lastPosition);
                 assertEquals(cursor.getReadPosition(), nextPosition);
+                assertEquals(cursor.getNumberOfEntries(), 0L);
             }
 
             @Override