Return-Path: Delivered-To: apmail-hbase-dev-archive@www.apache.org Received: (qmail 51448 invoked from network); 1 Mar 2011 22:52:37 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 1 Mar 2011 22:52:37 -0000 Received: (qmail 98560 invoked by uid 500); 1 Mar 2011 22:52:37 -0000 Delivered-To: apmail-hbase-dev-archive@hbase.apache.org Received: (qmail 98499 invoked by uid 500); 1 Mar 2011 22:52:36 -0000 Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list dev@hbase.apache.org Received: (qmail 98491 invoked by uid 99); 1 Mar 2011 22:52:36 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 01 Mar 2011 22:52:36 +0000 X-ASF-Spam-Status: No, hits=2.2 required=5.0 tests=FREEMAIL_FROM,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of ghelmling@gmail.com designates 209.85.220.169 as permitted sender) Received: from [209.85.220.169] (HELO mail-vx0-f169.google.com) (209.85.220.169) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 01 Mar 2011 22:52:31 +0000 Received: by vxc38 with SMTP id 38so6312258vxc.14 for ; Tue, 01 Mar 2011 14:52:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=w98R7CnG6RzaWypvN+56UiyccnvzJ89cY3ighzBB+Xw=; b=K3fBC/nHXNYG5rSRmshoHGMqM9w/9XJU0RG5S9gKF2ii3BB4wB0l5TaZZS0j+aGfT/ o6LwP0Q9CmtZWFkYv1N5H7LezZuUC7AVCSm5PYhK3VknyWAJ5QWE4BWbh/xks1Tb6D/c qy0YVB4skpdcJ6JtNVdumPBiCICL2NpQ0s29w= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=XU945FMOqDVwOApPR5zkMMu1slwElK8aOQIZdrejn4mtYoxWyut/iqxWxYNxmCG412 FitVXzsSI5nblIfK4bUBYQCDoo75NrdOB5a3Elw+FBhAQ8ITBOgLwTQJIdQgzXEV6OdQ lhcS4YhvecOloWJeL+qlC+VkQbOK6I6TydgT4= MIME-Version: 1.0 Received: by 10.52.99.198 with SMTP id es6mr12471764vdb.3.1299019929897; Tue, 01 Mar 2011 14:52:09 -0800 (PST) Received: by 10.52.158.4 with HTTP; Tue, 1 Mar 2011 14:52:09 -0800 (PST) In-Reply-To: References: Date: Tue, 1 Mar 2011 14:52:09 -0800 Message-ID: Subject: Re: Coprocessor tax? From: Gary Helmling To: Ryan Rawson Cc: dev@hbase.apache.org Content-Type: multipart/alternative; boundary=20cf3071c6f87a4be5049d73a574 --20cf3071c6f87a4be5049d73a574 Content-Type: text/plain; charset=ISO-8859-1 Created HBASE-3587: https://issues.apache.org/jira/browse/HBASE-3587 Please comment/correct if I left anything out or summed it up wrong. On Tue, Mar 1, 2011 at 12:59 PM, Gary Helmling wrote: > Sure, even better (it's doing the locking for you). > > > On Tue, Mar 1, 2011 at 12:49 PM, Ryan Rawson 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" 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 >> 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 >> >> 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 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 >> >> >> >> >> > >> >> >> > > --20cf3071c6f87a4be5049d73a574--