You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Jean-Daniel Cryans <jd...@apache.org> on 2010/06/09 01:04:06 UTC

Review Request: LeaseStillHeldException totally ignored by RS, wrongly named

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/164/
-----------------------------------------------------------

Review request for hbase.


Summary
-------

Patch that first adds a new testing facility to send exceptions to region servers when they report in. 
Then I added YouAreDeadException and PleaseHoldException that replaces the ambiguous lease still held exception, and both are treated differently.
Finally, I added a very simple test that makes sure that telling a RS that its dead will actually end up in killing it.


This addresses bug HBASE-2691.


Diffs
-----

  /trunk/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java PRE-CREATION 
  /trunk/src/main/java/org/apache/hadoop/hbase/YouAreDeadException.java PRE-CREATION 
  /trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 952836 
  /trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 952836 
  /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 952836 
  /trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 952836 
  /trunk/src/test/java/org/apache/hadoop/hbase/TestYouAreDead.java PRE-CREATION 

Diff: http://review.hbase.org/r/164/diff


Testing
-------


Thanks,

Jean-Daniel


Re: Review Request: LeaseStillHeldException totally ignored by RS, wrongly named

Posted by Jean-Daniel Cryans <jd...@apache.org>.

> On 2010-06-08 16:18:09, stack wrote:
> > /trunk/src/test/java/org/apache/hadoop/hbase/TestYouAreDead.java, line 27
> > <http://review.hbase.org/r/164/diff/1/?file=1103#file1103line27>
> >
> >     These single test classes are killing us...regards overall unit tests times... can we not put it into an existing test class....?

Will be all merged into TestMasterWrongRS, that I rename to TestKillingServersFromMaster


- Jean-Daniel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/164/#review160
-----------------------------------------------------------


On 2010-06-08 16:04:06, Jean-Daniel Cryans wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/164/
> -----------------------------------------------------------
> 
> (Updated 2010-06-08 16:04:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Patch that first adds a new testing facility to send exceptions to region servers when they report in. 
> Then I added YouAreDeadException and PleaseHoldException that replaces the ambiguous lease still held exception, and both are treated differently.
> Finally, I added a very simple test that makes sure that telling a RS that its dead will actually end up in killing it.
> 
> 
> This addresses bug HBASE-2691.
> 
> 
> Diffs
> -----
> 
>   /trunk/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java PRE-CREATION 
>   /trunk/src/main/java/org/apache/hadoop/hbase/YouAreDeadException.java PRE-CREATION 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 952836 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 952836 
>   /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 952836 
>   /trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 952836 
>   /trunk/src/test/java/org/apache/hadoop/hbase/TestYouAreDead.java PRE-CREATION 
> 
> Diff: http://review.hbase.org/r/164/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jean-Daniel
> 
>


Re: Review Request: LeaseStillHeldException totally ignored by RS, wrongly named

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/164/#review160
-----------------------------------------------------------

Ship it!


+1 after making the mostly javadoc changes suggested below.


/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.hbase.org/r/164/#comment793>

    Javadoc missing here... you need to explain the bit about how you can send a exception back to RS



/trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
<http://review.hbase.org/r/164/#comment794>

    Fatten the message passed back to the regionserver.



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/164/#comment795>

    Not needed (after chatting w/ j-d)



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/164/#comment796>

    Same here



/trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
<http://review.hbase.org/r/164/#comment797>

    Remove



/trunk/src/test/java/org/apache/hadoop/hbase/TestYouAreDead.java
<http://review.hbase.org/r/164/#comment799>

    These single test classes are killing us...regards overall unit tests times... can we not put it into an existing test class....?


- stack


On 2010-06-08 16:04:06, Jean-Daniel Cryans wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/164/
> -----------------------------------------------------------
> 
> (Updated 2010-06-08 16:04:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Patch that first adds a new testing facility to send exceptions to region servers when they report in. 
> Then I added YouAreDeadException and PleaseHoldException that replaces the ambiguous lease still held exception, and both are treated differently.
> Finally, I added a very simple test that makes sure that telling a RS that its dead will actually end up in killing it.
> 
> 
> This addresses bug HBASE-2691.
> 
> 
> Diffs
> -----
> 
>   /trunk/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java PRE-CREATION 
>   /trunk/src/main/java/org/apache/hadoop/hbase/YouAreDeadException.java PRE-CREATION 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 952836 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 952836 
>   /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 952836 
>   /trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 952836 
>   /trunk/src/test/java/org/apache/hadoop/hbase/TestYouAreDead.java PRE-CREATION 
> 
> Diff: http://review.hbase.org/r/164/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jean-Daniel
> 
>