hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-17653) HBASE-17624 rsgroup synchronizations will (distributed) deadlock
Date Thu, 16 Feb 2017 17:42:41 GMT

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

stack commented on HBASE-17653:
-------------------------------

[~toffer] Here is feedback that made me dig in on synchronization to try and tell a coherent
story around it. From [~busbey] internal review... as part of an attempt at a backport:

{code}
hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
Line 2:

nit: incorrect header.
Line 348:

nit: doesn't the synchronized above mean only one balance will run at a time? so these RIT
would be from e.g. server failure? Couldn't those happen after this point?
Line 405:

Throughout this class we're inconsistent with locking on the object returned from this call.
From reading both this class' use, the interface RSGroupInfoManager, and the default implementation
it's not clear to me what correct synchronized handling is supposed to look like.

However, we synchronize for removing groups but not adding them and sometimes when retrieving
them, so my intuition is that something is incorrect.
{code}

and.....


{code}
hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java
Line 2:

nit: incorrect header
Line 148:

what are we trying to protect by synchronizing these methods?

if it's rsGroupMap, we still have methods that make use of the map without a proper synchronization
call.

reading through things, I think it's trying to keep updates to the reference for rsGroupMap
atomic (since I *think* all the things written there are unmodifiable, which would make the
unsynchronized access safe). It would be much clearer if we did that via an AtomicReference
or atleast documented that this is what our intention is for all these synchronized methods.
{code}

Then, in rb [~Apache9] questioned synchronization here https://reviews.apache.org/r/56570/diff/1?file=1630642#file1630642line221
 RSGroupInfoManagerImpl is used by RSGroupAdminServer. It holds the monitor for RSGroupInfoManagerImpl
across a set of operations making the reviewer think this method does not need synchronization.

Let me look back at the original and with your readers-should-not-be-blocked prescription,
come up w/ a patch for here...

> HBASE-17624 rsgroup synchronizations will (distributed) deadlock
> ----------------------------------------------------------------
>
>                 Key: HBASE-17653
>                 URL: https://issues.apache.org/jira/browse/HBASE-17653
>             Project: HBase
>          Issue Type: Bug
>          Components: rsgroup
>            Reporter: stack
>            Assignee: stack
>
> Follow-on from HBASE-17624. HBASE-17624 made it so one thread only has access to the
rsgroup administrator. In tail of HBASE-17624 [~toffer] describes scenario under which we
 may end up in a deadlock (distributed). Let me repeat [~toffer] comment...
> {code}
> Both read/write access can't be single threaded. Consider the situation:
> 1. move_rsgroup_servers is called
> 2. while #1 is happening rsgroup region is in transition (rpc thread in #1 holds monitor
lock)
> 3. while #2 is happening meta is in transition.
> Balancer tries to figure out plan for meta region tries to get monitor lock but can't.
rpc thread task won't release monitor lock since rsgroup region never gets assigned. rsgroup
region never gets assigned because it can't update meta with new state.
> There's a good chance this can be reproduce just by moving both rsgroup and meta region
onto the same RS and call move_rsgoup_servers on the same RS.
> A bunch different actors will query from group affiliation so we can't have writes block
reads.
> ....
> In the code prior to this patch the getter methods that retrieve group information (getRSGroup,
ofTable, OfServer, etc) don't require the monitor lock so the deadlock cycle is broken.
> ....
> The methods that does mutations and updates to zk and hbase:rsgroup are synchronized
appropriately. Point me to where the incoherence is?
> {code}
> This issue is about testing/fixing/restoring rsgroup access. Will be back.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message