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/10/14 09:46:49 UTC

[GitHub] [pulsar] nicoloboschi opened a new pull request #12364: [managedledger] NPE on OpAddEntry while ManagedLedger is closing

nicoloboschi opened a new pull request #12364:
URL: https://github.com/apache/pulsar/pull/12364


   ### Motivation
   
   The test `ManagedLedgerBkTest#managedLedgerClosed` closes `ManagedLedger` object between some `asyncAddEntry` operations and sometimes it fails with this error
   
   ```
   2021-10-14T10:21:58,294+0200 ERROR [BookKeeperClientWorker-OrderedExecutor-0-0] o.a.b.c.u.SafeRunnable@38 - Unexpected throwable caught  {}
   java.lang.NullPointerException: Cannot invoke "org.apache.bookkeeper.client.LedgerHandle.getId()" because "this.ledger" is null
   	at org.apache.bookkeeper.mledger.impl.OpAddEntry.addComplete(OpAddEntry.java:155) ~[classes/:?]
   	at org.apache.bookkeeper.client.AsyncCallback$AddCallback.addCompleteWithLatency(AsyncCallback.java:92) ~[bookkeeper-server-4.14.2.jar:4.14.2]
   	at org.apache.bookkeeper.client.PendingAddOp.submitCallback(PendingAddOp.java:431) ~[bookkeeper-server-4.14.2.jar:4.14.2]
   	at org.apache.bookkeeper.client.LedgerHandle.errorOutPendingAdds(LedgerHandle.java:1799) ~[bookkeeper-server-4.14.2.jar:4.14.2]
   	at org.apache.bookkeeper.client.LedgerHandle$5.safeRun(LedgerHandle.java:574) ~[bookkeeper-server-4.14.2.jar:4.14.2]
   	at org.apache.bookkeeper.common.util.SafeRunnable.run(SafeRunnable.java:36) [bookkeeper-common-4.14.2.jar:4.14.2]
   	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130) [?:?]
   	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630) [?:?]
   	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [netty-common-4.1.68.Final.jar:4.1.68.Final]
   	at java.lang.Thread.run(Thread.java:832) [?:?]
   ```
   
   I figured out that during the closing of the `ManagedLedger`, the pending `OpAddEntry` objects are set to failed and recycled. During these operations, BK ledger has been closed and it tries to make `PendingAddOp` objects to call their callbacks as well. So we end up with, for the first entry added after the ManagedLedger closing procedure:
   - failed callback is called (`AddEntryCallback#addFailed`)
   - the Java object is recycled ( `OpAddEntry#recycle` )
   - BK calls the callback which expect the object to be "running"  (`AddCallback#addComplete`)
   
   ### Modifications
   
   When ManagedLedger signals to the OpAddEntry to fail, now the state is updated to `CLOSED`. In this way the BK callback has no effect, the object is correctly recycled and the "failed" callback is correctly triggered.
   
   After this modification, the `ManagedLedgerBkTest#managedLedgerClosed` test has never failed locally. All managed-ledgers tests pass locally.
   
   No more tests needed since `ManagedLedgerBkTest#managedLedgerClosed` already tests this behaviour (I could make more precise test (by using mockito) but I found out that is pretty hard and IMHO not really needed)
   
   ### Verifying this change
   
   This change is already covered by existing tests, such as `ManagedLedgerBkTest`.
   
   ### 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
   
   Check the box below and label this PR (if you have committer privilege).
   
   Need to update docs? 
   [x] no-need-doc 
    


-- 
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] nicoloboschi commented on pull request #12364: [managedledger] NPE on OpAddEntry while ManagedLedger is closing

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


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

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

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



[GitHub] [pulsar] nicoloboschi commented on pull request #12364: [managedledger] NPE on OpAddEntry while ManagedLedger is closing

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


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

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

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



[GitHub] [pulsar] nicoloboschi commented on pull request #12364: [managedledger] NPE on OpAddEntry while ManagedLedger is closing

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


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

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

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



[GitHub] [pulsar] nicoloboschi commented on pull request #12364: [managedledger] NPE on OpAddEntry while ManagedLedger is closing

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


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

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

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