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/05/20 18:25:59 UTC
[GitHub] [pulsar] merlimat opened a new pull request #10659: Ensure all the ReadHandle gets properly closed on cache invalidation
merlimat opened a new pull request #10659:
URL: https://github.com/apache/pulsar/pull/10659
### Motivation
The `ReadHandle` instances used in ManagedLedger are not properly closed on either cache invalidation or when the managed ledger instance is closed.
While this does not cause any issue with regular BK LedgerHandle instances (since there are no resources to dispose of and the objects just get garbage collected), this is a problem with some of the offloader implementations which might keep file descriptions, sockets or other resources open.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on pull request #10659: Ensure all the ReadHandle gets properly closed on cache invalidation
Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #10659:
URL: https://github.com/apache/pulsar/pull/10659#issuecomment-846414153
@eolivelli Good point. The test is failing because of a thread deadlock in the test:
```
"test-OrderedScheduler-0-0" prio=5 tid=16 in Object.wait()
java.lang.Thread.State: WAITING (on object monitor)
at java.base@13.0.1/jdk.internal.misc.Unsafe.park(Native Method)
at java.base@13.0.1/java.util.concurrent.locks.LockSupport.park(LockSupport.java:194)
at java.base@13.0.1/java.util.concurrent.locks.StampedLock.acquireWrite(StampedLock.java:1329)
at java.base@13.0.1/java.util.concurrent.locks.StampedLock.writeLock(StampedLock.java:475)
at app//org.apache.pulsar.common.util.collections.ConcurrentLongHashMap$Section.remove(ConcurrentLongHashMap.java:340)
at app//org.apache.pulsar.common.util.collections.ConcurrentLongHashMap$Section.access$300(ConcurrentLongHashMap.java:194)
at app//org.apache.pulsar.common.util.collections.ConcurrentLongHashMap.remove(ConcurrentLongHashMap.java:141)
at app//org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.invalidateReadHandle(ManagedLedgerImpl.java:1619)
at app//org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.lambda$asyncClose$9(ManagedLedgerImpl.java:1239)
at app//org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl$$Lambda$250/0x0000000800d7c840.accept(Unknown Source)
at app//org.apache.pulsar.common.util.collections.ConcurrentLongHashMap$Section.forEach(ConcurrentLongHashMap.java:431)
at app//org.apache.pulsar.common.util.collections.ConcurrentLongHashMap.forEach(ConcurrentLongHashMap.java:164)
at app//org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.lambda$asyncClose$10(ManagedLedgerImpl.java:1238)
at app//org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl$$Lambda$249/0x0000000800d7c040.closeComplete(Unknown Source)
at app//org.apache.bookkeeper.client.PulsarMockLedgerHandle.lambda$asyncClose$1(PulsarMockLedgerHandle.java:96)
at app//org.apache.bookkeeper.client.PulsarMockLedgerHandle$$Lambda$229/0x0000000800d71040.accept(Unknown Source)
```
This is because the fix to `ConcurrentLongHashMap` (#9787) never got back ported to 2.7. Cherry-picked and the branch is working now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #10659: Ensure all the ReadHandle gets properly closed on cache invalidation
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10659:
URL: https://github.com/apache/pulsar/pull/10659#issuecomment-846399632
FYI This commit on branch-2.7 make test ManagedCursorTest hang.
Probably there is some missing dependency.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat merged pull request #10659: Ensure all the ReadHandle gets properly closed on cache invalidation
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #10659:
URL: https://github.com/apache/pulsar/pull/10659
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #10659: Ensure all the ReadHandle gets properly closed on cache invalidation
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10659:
URL: https://github.com/apache/pulsar/pull/10659#issuecomment-846416270
@merlimat
Thanks.
I wasn't able to find the missing commit
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] addisonj commented on pull request #10659: Ensure all the ReadHandle gets properly closed on cache invalidation
Posted by GitBox <gi...@apache.org>.
addisonj commented on pull request #10659:
URL: https://github.com/apache/pulsar/pull/10659#issuecomment-845509063
:clap: this looks great
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org