hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Virag Kothari (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-11290) Unlock RegionStates
Date Fri, 12 Sep 2014 22:47:34 GMT

    [ https://issues.apache.org/jira/browse/HBASE-11290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14132242#comment-14132242
] 

Virag Kothari commented on HBASE-11290:
---------------------------------------

Thanks for the review [~mantonov]
bq. LockFreeCache name implies that it's "cache (i.e. structure from where elements could
be evicted without any harm when expired or cache is overfilled) which is lock free", which
isn't exactly how it's used? I mean - the class itself is suited to serve as a set of named
lock objects, and it can't arbitrarily evict elements from it?

I named it 'lockfree' as we are retrieving elements from cache without obtaining lock. But
seems that is confusing as we are serving lock objects from cache. How about the name as LockCache?
. We need to evict elements otherwise the map may grow unbounded. In next version, I plan
to add Guava's cachebuilder with soft values instead of the CHM. 

bq. to be precise, not just setters updating multiple collections grab region lock, but readers
who want to read consistently from several ones, right? Like isRegionOnline()? Also this list
could contain RW-locks, right? As further improvements. Now guys like #isRegionOnline() can't
run in parallel querying about the same region from multiple threads, although they could
with RW lock.

Yep, for some readers reading several collections we are also grabbing region locks. Agree
that RW-locks can be a future optimization. 

bq. for ops like #regionOnline(), does it make sense to have the server-keyed locks, and operations
involving both HRI and server (hence needed to access/update serverHoldings) would first grab
lock for HRI, then lock for ServerName to work with server holdings? Seems more consistent?

Yeah, I was planning to add server locks  initially. But for methods like regionOnline(),
concurrent put and concurrent remove seems to be enough for server keyed structures like serverHoldings().
Also, we have to be more careful about deadlock type of situation when we deal with  two types
of lock (region and server). So, I thought about avoiding serverlocks for now.

bq. #logSplit made not synchronized now, we don't need locks to iterate over lastAssignments
map or to access processedServers?

The logSplit() is called by SSH and multiple SSH should not call logSplit() for the same ServerName.
Even in a case they do, I dont see any issue with multiple threads overlapping for this method.

bq. in #serverOffline do we still need notifyAll call at the last lines?

We are doing a wait using object lock somewhere. So kept this as is.

bq. In AM, #addPlan() grabs locks for region name before putting into regionPlans, but #addPlans()
doesn't get any locks? It assumed the lock is already grabbed for all those regions?

 I thought CHM.putAll is atomic but its not the case. Let me see whether we need locks here.

bq. in #processServerShutdown we don't need locks on regionPlans?

The iterator over regionPlans is weakly consistent and other threads might update/access the
map, but that seems to be ok here.




> Unlock RegionStates
> -------------------
>
>                 Key: HBASE-11290
>                 URL: https://issues.apache.org/jira/browse/HBASE-11290
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Francis Liu
>            Assignee: Virag Kothari
>         Attachments: HBASE-11290-0.98.patch, HBASE-11290.draft.patch
>
>
> Even though RegionStates is a highly accessed data structure in HMaster. Most of it's
methods are synchronized. Which limits concurrency. Even simply making some of the getters
non-synchronized by using concurrent data structures has helped with region assignments. We
can go as simple as this approach or create locks per region or a bucket lock per region bucket.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message