[
https://issues.apache.org/jira/browse/HBASE-2697?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12895052#action_12895052
]
HBase Review Board commented on HBASE-2697:
-------------------------------------------
Message from: "Jean-Daniel Cryans" <[email protected]>
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/484/#review636
-----------------------------------------------------------
First pass, still trying to understand what's going on
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java
<http://review.cloudera.org/r/484/#comment2393>
unused?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java
<http://review.cloudera.org/r/484/#comment2383>
Possible NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java
<http://review.cloudera.org/r/484/#comment2388>
Possible NPE, server can be null
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java
<http://review.cloudera.org/r/484/#comment2384>
Possible NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java
<http://review.cloudera.org/r/484/#comment2385>
Possible NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2386>
Possible NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2387>
Possible NPE, metaServer can be null
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2389>
Possible NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2397>
javadoc for this method and the next one
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2401>
This method and a few other ones in this class looks almost the same, I
think there's a refactoring potential
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2390>
Possible NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2398>
Somehow this line looks weird to me, not sure if it's how you build the row
or that you don't use a final for ",,"
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2391>
NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<http://review.cloudera.org/r/484/#comment2392>
NPE
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
<http://review.cloudera.org/r/484/#comment2402>
could enclose in a ifDebugEnabled
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/484/#comment2404>
Javadoc looks anemic compared to the importance of that class
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/484/#comment2405>
Could you handle if regionState is null at this level?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/484/#comment2406>
Is this code coming back later?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/484/#comment2407>
Looks fishy, what happens if anyone uses that state outside of that method?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/484/#comment2409>
Any investigation needed?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/484/#comment2410>
What happens if it never returns? Also I wonder if it's feasible that the
assignment somehow ends up waiting on itself because of region server failure
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java
<http://review.cloudera.org/r/484/#comment2412>
I see red boxes :)
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java
<http://review.cloudera.org/r/484/#comment2413>
red
- Jean-Daniel
> Implement new open/close logic in handlers and stop using heartbeats for
> open/close messages
> --------------------------------------------------------------------------------------------
>
> Key: HBASE-2697
> URL: https://issues.apache.org/jira/browse/HBASE-2697
> Project: HBase
> Issue Type: Sub-task
> Components: ipc, master, regionserver
> Reporter: Jonathan Gray
> Assignee: Jonathan Gray
> Priority: Critical
> Fix For: 0.90.0
>
>
> This issue is doing the meat of what HBASE-2485 is about and continues what
> was started in HBASE-2694 after some code cleanup to make life easier.
> This deals with no longer piggybacking messages from Master to RegionServers
> on heartbeat responses and instead sending direct unsolicited messages. This
> also deals with moving the open/close logic fully into handlers and removing
> the existing open/close code on both the RS and M sides. There may also be
> some changes to the master in-memory state of regions in transition. The new
> load balancer will probably be introduced with this issue but not fully
> integrated yet.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
