Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Broker] Async read entries with max size bytes #9532

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Feb 8, 2021

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 asyncReadEntries(int numberOfEntriesToRead, long maxSizeBytes, ReadEntriesCallback callback, Object ctx, PositionImpl maxPosition) in the interface ManagedCursor.
  2. Add a new method asyncReadEntries(int numberOfEntriesToRead, 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)

@merlimat merlimat added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Feb 8, 2021
@merlimat merlimat added this to the 2.8.0 milestone Feb 8, 2021
@gaoran10
Copy link
Contributor Author

gaoran10 commented Feb 9, 2021

/pulsarbot run-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Test(timeOut = 20000)
void testAsyncReadWithMaxSizeByte() throws Exception {
ManagedLedger ledger = factory.open("testAsyncReadWithMaxSizeByte");
ManagedCursor cursor = ledger.openCursor("c1");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@merlimat merlimat merged commit 34fdb67 into apache:master Feb 9, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Feb 18, 2021
codelipenghui pushed a commit that referenced this pull request Feb 18, 2021
…ies with max size bytes. (#9532)

(cherry picked from commit 34fdb67)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.1 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants