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/02/08 18:31:25 UTC

[GitHub] [pulsar] gaoran10 opened a new pull request #9532: [Broker] Async read entries with max size bytes

gaoran10 opened a new pull request #9532:
URL: https://github.com/apache/pulsar/pull/9532


   ### Motivation
   
   Currently, we could specify the param `maxSizeBytes` when reading entries by the method `asyncReadEntriesOrWait`, but this method will be blocked when there are no entries to read. So I want to add a new method to read entries with param `maxSizeBytes` and return data immediately if there are no entries to read.
   
   ### Modifications
   
   1. Add a new method `asyncReadEntriesOrWait(int maxEntries, long maxSizeBytes, ReadEntriesCallback callback, Object ctx,
                                   PositionImpl maxPosition)`  in the interface `ManagedCursor`.
   2. Add a new method `asyncReadEntriesOrWait(int maxEntries, long maxSizeBytes, ReadEntriesCallback callback, Object ctx,
                                   PositionImpl maxPosition)`  in the interface `ReadOnlyCursor`.
   
   ### Verifying this change
   
   This change added tests and can be verified as follows:
   
     - *Added a test for reading entries with max size bytes*
   
   ### 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: (yes)
     - 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)
   
   


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

[GitHub] [pulsar] gaoran10 commented on pull request #9532: [Broker] Async read entries with max size bytes

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


   /pulsarbot run-failure-checks


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

[GitHub] [pulsar] merlimat merged pull request #9532: [Broker] Async read entries with max size bytes

Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #9532:
URL: https://github.com/apache/pulsar/pull/9532


   


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

[GitHub] [pulsar] eolivelli commented on a change in pull request #9532: [Broker] Async read entries with max size bytes

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



##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java
##########
@@ -463,6 +463,42 @@ public void readEntriesFailed(ManagedLedgerException exception, Object ctx) {
         counter.await();
     }
 
+    @Test(timeOut = 20000)
+    void testAsyncReadWithMaxSizeByte() throws Exception {
+        ManagedLedger ledger = factory.open("testAsyncReadWithMaxSizeByte");
+        ManagedCursor cursor = ledger.openCursor("c1");

Review comment:
       we aren't closing this resources.
   Do you think we can add @Cleanup annotations in this file in all of the tests ?
   
   (not blocker for me)




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

[GitHub] [pulsar] merlimat commented on a change in pull request #9532: [Broker] Async read entries with max size bytes

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



##########
File path: managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java
##########
@@ -463,6 +463,42 @@ public void readEntriesFailed(ManagedLedgerException exception, Object ctx) {
         counter.await();
     }
 
+    @Test(timeOut = 20000)
+    void testAsyncReadWithMaxSizeByte() throws Exception {
+        ManagedLedger ledger = factory.open("testAsyncReadWithMaxSizeByte");
+        ManagedCursor cursor = ledger.openCursor("c1");

Review comment:
       That's ok in these tests, the factory gets re-created after each test and it automatically closes everything in the shutdown.




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