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

[issue #3975] Bugfix NPE on non durable consumer #3988

Merged
merged 1 commit into from
Apr 9, 2019

Conversation

lovelle
Copy link
Contributor

@lovelle lovelle commented Apr 5, 2019

Motivation

Trying to fix #3975

When a reset of a cursor is performed with some timestamp on a non-durable
consumer the message finder will fail with null pointer exception due to
cursor.getName() being null.

Modifications

  • Add method overloading for newNonDurableCursor() with subscription name.
  • Fix method getNonDurableSubscription to call newNonDurableCursor() with
    proper subscription name
  • Add test to assert issue.

Verifying this change

  • Make sure that the change passes the CI checks.

@@ -193,6 +193,7 @@
* @return the new NonDurableCursor
*/
ManagedCursor newNonDurableCursor(Position startCursorPosition) throws ManagedLedgerException;
ManagedCursor newNonDurableCursor(Position startPosition, String subscriptionName) throws ManagedLedgerException;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that the correct action here would be to modify existing newNonDurableCursor instead of adding a new one, but I did that way in order to leave this change as easy as possible, please let me know what you think.

@lovelle
Copy link
Contributor Author

lovelle commented Apr 5, 2019

Hi @ConcurrencyPractitioner please take a look at this pull, you might face this issue adding tests on #3983 to Reader with reset by timestamp

@lovelle lovelle force-pushed the bugfix/npe_on_non_durable_consumer branch from bc74097 to 3a879ab Compare April 5, 2019 18:29
*Motivation*

Trying to fix apache#3975

When a reset of a cursor is performed with some timestamp on a non-durable
consumer the message finder will fail with null pointer exception due to
`cursor.getName()` being null.

*Modifications*

  - Add method overloading for `newNonDurableCursor()` with subscription name.
  - Fix method getNonDurableSubscription to call `newNonDurableCursor()` with
    proper subscription name
  - Add test to assert issue.
@lovelle lovelle force-pushed the bugfix/npe_on_non_durable_consumer branch from 3a879ab to 1434b02 Compare April 5, 2019 19:35
@sijie sijie added area/broker type/bug The PR fixed a bug or issue reported a bug labels Apr 8, 2019
@sijie sijie added this to the 2.3.2 milestone Apr 8, 2019
@merlimat
Copy link
Contributor

merlimat commented Apr 8, 2019

run integration tests

@merlimat merlimat merged commit 372575a into apache:master Apr 9, 2019
@lovelle lovelle deleted the bugfix/npe_on_non_durable_consumer branch April 9, 2019 02:11
merlimat pushed a commit that referenced this pull request May 19, 2019
*Motivation*

Trying to fix #3975

When a reset of a cursor is performed with some timestamp on a non-durable
consumer the message finder will fail with null pointer exception due to
`cursor.getName()` being null.

*Modifications*

  - Add method overloading for `newNonDurableCursor()` with subscription name.
  - Fix method getNonDurableSubscription to call `newNonDurableCursor()` with
    proper subscription name
  - Add test to assert issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to reset subscription: Message id was not present
3 participants