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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
