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-7390) Add extra test cases for assignement on the region server and fix the related issues
Date Thu, 20 Dec 2012 19:49:14 GMT

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

stack commented on HBASE-7390:
------------------------------

On the patch, what Sergey said about different closeRegion returns.  Could the one that throws
an exception be private?

Would putIfAbsent be easier to read if it took an enum rather than a boolean to say whether
open or close?  For example, what was there previous was opaque enough and I appreciate your
collecting together tests into a single method but the below could be better:

-    if (this.regionsInTransitionInRS.containsKey(region.getEncodedNameAsBytes())) {
-      LOG.warn("Received close for region we are already opening or closing; " +
-        region.getEncodedName());
+    if (putIfAbsent(region, false)){
+      // We're already closing this region.


This below continue is right?

+          builder.addOpeningState(RegionOpeningState.OPENED);
+          continue;


I see you do it again later if we continue... so above looks like it makes sense.

Review comments.  Some error.  e.g. +   * @param expectedVersion expecpted version og the
znode  It looks like klingon.

I appreciate these changes:

-      String regionName)
+      String encodedRegionName)

I always have to back up to find what what is needed making these calls.  I suppose we should
make an encodedRegionName type one of these days.

Do we need to do this?

+    zkw.sync(encoded);

Its to be 'sure'...  Its expensive though...

Nice test.

Can you say more on this @nkeywal "This code there is very very smart: if there is a open
in progress, it changes the internal state to close, then raises an exception through the
call to checkIfRegionInTransition. As we changed the state, we will have, if we were currently
opening, a message saying that we were trying to close a region already closing."

I'm not sure I follow.

The patch looks great, cleaning up fuzzy state. 

Let me get Jimmy to take a looksee too.  He LOVEs this stuff.  [~jxiang] Any chance of your
taking a look here boss?










                
> Add extra test cases for assignement on the region server and fix the related issues
> ------------------------------------------------------------------------------------
>
>                 Key: HBASE-7390
>                 URL: https://issues.apache.org/jira/browse/HBASE-7390
>             Project: HBase
>          Issue Type: Bug
>          Components: Region Assignment, regionserver
>    Affects Versions: 0.96.0
>            Reporter: nkeywal
>            Assignee: nkeywal
>             Fix For: 0.96.0
>
>         Attachments: 7390.v1.patch, 7390.v2.patch, 7390.v3.patch, 7390.v4.patch, assignment_zk_states.jpg
>
>
> We don't have a lot of tests on the region server itself.
> Here are some.
> Some of them are failing, feedback welcome.
> See as well the attached state diagram for the ZK nodes on assignment.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message