Sure, even better (it's doing the locking for you).
On Tue, Mar 1, 2011 at 12:49 PM, Ryan Rawson <[email protected]> wrote: > I don't think we need a lock even for updating, check it copy on write > array list. > On Mar 1, 2011 12:45 PM, "Gary Helmling" <[email protected]> wrote: > > Yeah, I was just looking at the write lock references as well. > > > > I'm not sure RegionCoprocessorHost.preClose() would really need the write > > lock? As you say, there is still a race in HRegion.doClose() between > > preClose() completing and HRegion.lock.writeLock() being taken out, so > other > > methods could still be called after. > > > > RegionCoprocessorHost.postClose() occurs under the HRegion write lock, so > > any read lock operations would already have to have completed by this > point. > > So here we wouldn't really need the coprocessor write lock either? > > > > It seems like we could actually drop the coprocessor lock, since > > coprocessors are currently loaded prior to region open completing. > > > > Online coprocessor loading (not currently provided) could be handled in > the > > future by a lock just for loading, and creating a new coprocessor > collection > > and assigning when done. > > > > On Tue, Mar 1, 2011 at 12:08 PM, Ryan Rawson <[email protected]> wrote: > > > >> My own profiling shows that a read write lock can be up to 3-6% of the > >> CPU budget in our put/get query path. Adding another one if not > >> necessary would probably not be good. > >> > >> In fact in the region coprocessor the only thing the write lock is > >> used for is the preClose and postClose, but looking in the > >> implementation of those methods I don't really get why this is > >> necessary. The write lock ensures single thread access, but there is > >> nothing that prevents other threads from calling other methods AFTER > >> the postClose? > >> > >> -ryan > >> > >> On Tue, Mar 1, 2011 at 12:02 PM, Gary Helmling <[email protected]> > >> wrote: > >> > All the CoprocessorHost invocations should be wrapped in "if (cpHost > != > >> > null)". We could just added an extra check for whether any > coprocessors > >> are > >> > loaded -- "if (cpHost != null && cpHost.isActive())", something like > >> that? > >> > Or the CoprocessorHost methods could do this checking internally. > >> > > >> > Either way should be relatively easy to bypass the lock acquisition. > Is > >> > there much overhead to acquiring a read lock if the write lock is > never > >> > taken though? (just wondering) > >> > > >> > > >> > > >> > On Tue, Mar 1, 2011 at 11:51 AM, Stack <[email protected]> wrote: > >> > > >> >> So, I'm debugging something else but thread dumping I see a bunch of > >> this: > >> >> > >> >> > >> >> "IPC Server handler 6 on 61020" daemon prio=10 tid=0x00000000422d2800 > >> >> nid=0x7714 runnable [0x00007f1c5acea000] > >> >> java.lang.Thread.State: RUNNABLE > >> >> at > >> >> > >> > java.util.concurrent.locks.ReentrantReadWriteLock$Sync.fullTryAcquireShared(ReentrantReadWriteLock.java:434) > >> >> at > >> >> > >> > java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryAcquireShared(ReentrantReadWriteLock.java:404) > >> >> at > >> >> > >> > java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireShared(AbstractQueuedSynchronizer.java:1260) > >> >> at > >> >> > >> > java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.lock(ReentrantReadWriteLock.java:594) > >> >> at > >> >> > >> > org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost.prePut(RegionCoprocessorHost.java:532) > >> >> at > >> >> > >> > org.apache.hadoop.hbase.regionserver.HRegion.doMiniBatchPut(HRegion.java:1476) > >> >> at > >> >> org.apache.hadoop.hbase.regionserver.HRegion.put(HRegion.java:1454) > >> >> at > >> >> > >> > org.apache.hadoop.hbase.regionserver.HRegionServer.multi(HRegionServer.java:2652) > >> >> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > >> >> at > >> >> > >> > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) > >> >> at > >> >> > >> > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) > >> >> at java.lang.reflect.Method.invoke(Method.java:597) > >> >> at > >> >> > >> > org.apache.hadoop.hbase.ipc.WritableRpcEngine$Server.call(WritableRpcEngine.java:309) > >> >> at > >> >> > >> > org.apache.hadoop.hbase.ipc.HBaseServer$Handler.run(HBaseServer.java:1060) > >> >> > >> >> > >> >> Do others? I don't have any CPs loaded. I'm wondering if we can do > >> >> more to just avoid the CP codepath if no CPs loaded. > >> >> > >> >> St.Ack > >> >> > >> > > >> >
