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 2022/08/01 18:32:18 UTC
[GitHub] [pulsar] tjiuming opened a new pull request, #16909: fix NPE
tjiuming opened a new pull request, #16909:
URL: https://github.com/apache/pulsar/pull/16909
Fixes [#16907](https://github.com/apache/pulsar/issues/16907)
### Motivation
Fixes [#16907](https://github.com/apache/pulsar/issues/16907)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
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] tisonkun commented on pull request #16909: [fix][ml] fix NPE
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #16909:
URL: https://github.com/apache/pulsar/pull/16909#issuecomment-1202000970
@mattisonchao I found these lines:
https://github.com/apache/pulsar/blob/33dd4f2a7e8c78ff96ed2795fef4fd4ab35e3d89/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L773-L788
Not sure whether it's exactly the callback in issue.
--
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] mattisonchao commented on pull request #16909: [fix][ml] fix NPE
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #16909:
URL: https://github.com/apache/pulsar/pull/16909#issuecomment-1201988104
Yes, you are right. But I'm concerned why `entries` has a null element.
--
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] tjiuming commented on pull request #16909: fix NPE
Posted by GitBox <gi...@apache.org>.
tjiuming commented on PR #16909:
URL: https://github.com/apache/pulsar/pull/16909#issuecomment-1201983074
> I also met this problem before. Could you explain why it causes NPE here?
I believe there existing null element in the `entries`, but I don't think it caused by multi-threads adding.
```
private void asyncReadEntry0(ReadHandle lh, PositionImpl position, final ReadEntryCallback callback,
final Object ctx) {
if (log.isDebugEnabled()) {
log.debug("[{}] Reading entry ledger {}: {}", ml.getName(), lh.getId(), position.getEntryId());
}
EntryImpl entry = entries.get(position);
if (entry != null) {
EntryImpl cachedEntry = EntryImpl.create(entry);
entry.release();
manager.mlFactoryMBean.recordCacheHit(cachedEntry.getLength());
callback.readEntryComplete(cachedEntry, ctx);
} else {
lh.readAsync(position.getEntryId(), position.getEntryId()).thenAcceptAsync(
ledgerEntries -> {
try {
Iterator<LedgerEntry> iterator = ledgerEntries.iterator();
if (iterator.hasNext()) {
LedgerEntry ledgerEntry = iterator.next();
EntryImpl returnEntry = RangeEntryCacheManagerImpl.create(ledgerEntry, interceptor);
manager.mlFactoryMBean.recordCacheMiss(1, returnEntry.getLength());
ml.getMbean().addReadEntriesSample(1, returnEntry.getLength());
callback.readEntryComplete(returnEntry, ctx);
} else {
// got an empty sequence
callback.readEntryFailed(new ManagedLedgerException("Could not read given position"),
ctx);
}
} finally {
ledgerEntries.close();
}
}, ml.getExecutor().chooseThread(ml.getName())).exceptionally(exception -> {
ml.invalidateLedgerHandle(lh);
callback.readEntryFailed(createManagedLedgerException(exception), ctx);
return null;
});
}
}
```
`ml.getExecutor().chooseThread(ml.getName())` a ManagedLedger always use the same SingleThreadExecutor.
--
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] tjiuming closed pull request #16909: [fix][ml] fix NPE
Posted by GitBox <gi...@apache.org>.
tjiuming closed pull request #16909: [fix][ml] fix NPE
URL: https://github.com/apache/pulsar/pull/16909
--
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] mattisonchao commented on pull request #16909: fix NPE
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #16909:
URL: https://github.com/apache/pulsar/pull/16909#issuecomment-1201788261
I also met this problem before. Could you explain why it causes NPE here?
--
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] tjiuming commented on pull request #16909: [fix][ml] fix NPE
Posted by GitBox <gi...@apache.org>.
tjiuming commented on PR #16909:
URL: https://github.com/apache/pulsar/pull/16909#issuecomment-1202418920
> Yes, you are right. But I'm concerned why `entries` has a null element.
me to, I don't find anywhere return null value
--
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] AnonHxy commented on pull request #16909: [fix][ml] fix NPE
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #16909:
URL: https://github.com/apache/pulsar/pull/16909#issuecomment-1203519959
It dosen't fix the root cause bug. Maybe we could print a warn log if `entries` add a null value I think.
--
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