headers
stack | 1 Oct 2010 01:11

Re: Review Request: hbase-3019 Make bulk assignment on cluster startup run faster


> On 2010-09-30 15:05:13, Ted Yu wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line 644
> > <http://review.cloudera.org/r/926/diff/1/?file=13151#file13151line644>
> >
> >     The handling seems to be different from that on line 584.

Thanks for the review Ted.  Yeah, its different.  Thats what the '// TODO: Should we just abort in this case? 
Then we'll notice these' question was about.  I'll make them same on commit.  In both cases we'll abort the
server so we notice these illegal state changes the sooner.

- stack

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/926/#review1372
-----------------------------------------------------------

On 2010-09-30 12:55:33, stack wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/926/
> -----------------------------------------------------------
> 
> (Updated 2010-09-30 12:55:33)
> 
> 
> Review request for hbase.
>
(Continue reading)

Permalink | Reply | 0 comments |
headers
stack | 1 Oct 2010 01:11

Re: Review Request: hbase-3019 Make bulk assignment on cluster startup run faster


> On 2010-09-30 14:38:49, Jonathan Gray wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line 486
> > <http://review.cloudera.org/r/926/diff/1/?file=13151#file13151line486>
> >
> >     Have you seen this?  I guess this is kind of stuff that would happen on failed-over master?

No. Haven't seen this.  Its a warn message.  If we see 'em something is up.

> On 2010-09-30 14:38:49, Jonathan Gray wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line 604
> > <http://review.cloudera.org/r/926/diff/1/?file=13151#file13151line604>
> >
> >     So basic assumption here for startup is that master and no RS will fail.  I'm fine with that.
> >     
> >     This bulk assign only done on startup, right?

Yes.  Has to be up for this bulk assign.  I think its fine if it crashes thereafter... we'll soldier on.  To be
proved in a test.

> On 2010-09-30 14:38:49, Jonathan Gray wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line 612
> > <http://review.cloudera.org/r/926/diff/1/?file=13151#file13151line612>
> >
> >     No race here?  What if region server transitions to OPENING already before we get here?

Chatting on irc, i like your suggestion of moving this state setting to before the rpc call.

> On 2010-09-30 14:38:49, Jonathan Gray wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java, line 877
(Continue reading)

Permalink | Reply | 0 comments |
headers
Apache Hudson Server | 1 Oct 2010 01:41
Picon

Build failed in Hudson: HBase-TRUNK #1493

See <https://hudson.apache.org/hudson/job/HBase-TRUNK/1493/changes>

Changes:

[jgray] HBASE-3060 Disabling test until it gets fixed (part 2)

------------------------------------------
[...truncated 713 lines...]
A         src/main/ruby/shell/commands/scan.rb
A         src/main/ruby/shell/commands/version.rb
A         src/main/ruby/shell/commands/truncate.rb
A         src/main/ruby/shell/commands/compact.rb
A         src/main/ruby/shell/commands/enable.rb
A         src/main/ruby/shell/commands/count.rb
A         src/main/ruby/shell/commands/enable_region.rb
A         src/main/ruby/shell/commands/deleteall.rb
A         src/main/ruby/shell/commands/incr.rb
A         src/main/ruby/shell/commands/split.rb
A         src/main/ruby/shell/commands/delete.rb
A         src/main/ruby/shell/commands/disable.rb
A         src/main/ruby/shell/commands/create.rb
A         src/main/ruby/shell/commands/drop.rb
A         src/main/ruby/shell/commands/disable_region.rb
A         src/main/ruby/shell/commands/alter.rb
A         src/main/ruby/shell/commands/close_region.rb
A         src/main/ruby/shell/commands/put.rb
A         src/main/ruby/shell/commands/zk_dump.rb
A         src/main/ruby/shell/commands/list.rb
A         src/main/ruby/shell/commands/shutdown.rb
A         src/main/ruby/shell/commands.rb
(Continue reading)

Permalink | Reply | 2 comments |
headers
Gary Helmling | 1 Oct 2010 02:13
Picon
Gravatar

Re: Review Request: [HBASE-2321] [HBASE-2002] Add support for per-region dynamically registered RPC endpoints for coprocessors and allow configurable RPC client/server implementations


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

(Updated 2010-09-30 17:13:36.365310)

Review request for hbase, Andrew Purtell and Jonathan Gray.

Changes
-------

Update patch to latest trunk.

Addressed latest comments:
  - Batch.returning() now explicitly checks for null args and throws an NPE with message
  - HTable.getRowKeysInRange() -> HTable.getStartKeysInRange()

Summary
-------

This is really two separate patches in one, though with some overlapping changes.  If necessary I can split
them apart for separate review.  Please let me know if that would make review easier.

Part 1:
==============
Port over of HADOOP-6422 to the HBase RPC code.  The goal of this change is to allow alternate RPC
client/server implementations to be enabled through a simple configuration change.  Ultimately I would
like to use this to allow secure RPC to be enabled through configuration, while not blocking normal
(Continue reading)

Permalink | Reply | 15 comments |
headers
Gary Helmling | 1 Oct 2010 02:43
Picon
Gravatar

Re: Review Request: [HBASE-2321] [HBASE-2002] Add support for per-region dynamically registered RPC endpoints for coprocessors and allow configurable RPC client/server implementations


> On 2010-09-25 16:18:25, Andrew Purtell wrote:
> > src/main/java/org/apache/hadoop/hbase/client/HTable.java, line 1400
> > <http://review.cloudera.org/r/816/diff/5/?file=12533#file12533line1400>
> >
> >     Maybe for sake of clarity call this getStartKeysInRange?
> 
> Gary Helmling wrote:
>     Makes sense as well, will rename.
> 
> Himanshu Vashishtha wrote:
>     Gary: I meant this use case (might be irrelevant, considering scope of cp):
>     If I want to know count of number of rows in a range (Row A, Row B): with the help of this method, I can get the
starting row of all regions that are in this range but for the first (where you take the starting row of the
range: Row A in this case).
>     So, when the processing is done in the last region, one shd be aware of the last row in the range, Row B. This
processing will be done in the cp impl, but that impl will be same on all region servers, so that check will be
there for all regions (no?). 
>     It is entirely possible  that this use case is not the one to be supported by cp; or as I haven't really looked
at mlai's code thoroughly yet, might be missing something obvious.

A key point to understand in the HTable.exec() calls is that List<Row> and RowRange arguments are _only_
used to locate the regions against which we'll invoke the CoprocessorProtocol method.  Since
coprocessors run in place per-region, we need to somehow indicate the region/coprocessor instances
where the method should be invoked.

Note that the List<Row> or RowRange that were passed to HTable.exec() are _not_ made directly available to
the CoprocessorProtocol method invoked, and couldn't easily be passed without a different approach to
the framework.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Mingjie Lai | 1 Oct 2010 04:18
Picon

Re: Review Request: HBase-2001: Coprocessors: Colocate user code with regions


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

(Updated 2010-09-30 19:18:18.297399)

Review request for hbase.

Changes
-------

Fixes by review board comments

Summary
-------

The diff actually contains 2 seperate patches: HBase-2001 and the one for (HBASE-2002+HBASE-2321). The
reason is that HBase-2001's CommandTarget relies on HBASE-2002 + HBASE-2321 which patches are still
under review. I have to include Gary's HBASE-2002, HBASE-2321 with this diff, since reviewboard is so
powerful :) and it disallow my diff to be based on some unchecked in patch. 

Both HBase-2001 and the dynamic RPC stuff are quite big patches. Total number of lines are more than 7k. I
turned back and forth, but still don't have a good idea to create the patch in order to reduce the review
pain. However right now I'm putting the whole patch for all the 3 issues. Here the list of file which are only
related to coprocessor:

src/main/java/org/apache/hadoop/hbase/coprocessor/BaseCommandTarget.java
src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
(Continue reading)

Permalink | Reply | 19 comments |
headers
Apache Hudson Server | 1 Oct 2010 02:54
Picon

Build failed in Hudson: HBase-TRUNK #1494

See <https://hudson.apache.org/hudson/job/HBase-TRUNK/1494/changes>

Changes:

[stack] HBASE-3019 Make bulk assignment on cluster startup run faster

------------------------------------------
[...truncated 713 lines...]
A         src/main/ruby/shell/commands/scan.rb
A         src/main/ruby/shell/commands/version.rb
A         src/main/ruby/shell/commands/truncate.rb
A         src/main/ruby/shell/commands/compact.rb
A         src/main/ruby/shell/commands/enable.rb
A         src/main/ruby/shell/commands/count.rb
A         src/main/ruby/shell/commands/enable_region.rb
A         src/main/ruby/shell/commands/deleteall.rb
A         src/main/ruby/shell/commands/incr.rb
A         src/main/ruby/shell/commands/split.rb
A         src/main/ruby/shell/commands/delete.rb
A         src/main/ruby/shell/commands/disable.rb
A         src/main/ruby/shell/commands/create.rb
A         src/main/ruby/shell/commands/drop.rb
A         src/main/ruby/shell/commands/disable_region.rb
A         src/main/ruby/shell/commands/alter.rb
A         src/main/ruby/shell/commands/close_region.rb
A         src/main/ruby/shell/commands/put.rb
A         src/main/ruby/shell/commands/zk_dump.rb
A         src/main/ruby/shell/commands/list.rb
A         src/main/ruby/shell/commands/shutdown.rb
A         src/main/ruby/shell/commands.rb
(Continue reading)

Permalink | Reply | 1 comment |
headers
Jonathan Gray | 1 Oct 2010 19:00
Picon
Favicon

Re: Review Request: Add ability to have multiple Masters in LocalHBaseCluster for test writing


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

(Updated 2010-10-01 10:00:03.133377)

Review request for hbase, stack, Karthik Ranganathan, and Kannan Muthukkaruppan.

Changes
-------

Adds TestMasterFailover which passes!  Starts cluster w/ 3 masters.  Stops a backup master, then stops the
active master, and verifies the other backup becomes the active master.  Doesn't test w/ tables/data or
regions-in-transition but the test framework improvements work.

A few things that were changed:

- can now close backup masters
- can now close active master w/o shutting down cluster
- regionserver reconnects to new master when fail over

Summary
-------

To really be able to unit test the new master properly, we need to be able to have multiple masters running at
once within a single logical cluster.

Also exposes methods to get the currently active master and can access each individually in the same way
(Continue reading)

Permalink | Reply | 7 comments |
headers
Gary Helmling | 1 Oct 2010 22:48
Picon
Gravatar

Re: Review Request: [HBASE-2321] [HBASE-2002] Add support for per-region dynamically registered RPC endpoints for coprocessors and allow configurable RPC client/server implementations


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

(Updated 2010-10-01 13:48:46.472644)

Review request for hbase, Andrew Purtell and Jonathan Gray.

Changes
-------

Update with small fix for org.apache.hadoop.hbase.io.TestHeapSize, courtesy of mlai (thanks Mingjie).

Summary
-------

This is really two separate patches in one, though with some overlapping changes.  If necessary I can split
them apart for separate review.  Please let me know if that would make review easier.

Part 1:
==============
Port over of HADOOP-6422 to the HBase RPC code.  The goal of this change is to allow alternate RPC
client/server implementations to be enabled through a simple configuration change.  Ultimately I would
like to use this to allow secure RPC to be enabled through configuration, while not blocking normal
(current) RPC operation on non-secure Hadoop versions.

This portion of the patch abstracts out two interfaces from the RPC code:
(Continue reading)

Permalink | Reply | 14 comments |
headers
Ryan Rawson | 1 Oct 2010 22:54
Picon
Gravatar

Re: Review Request: [HBASE-2321] [HBASE-2002] Add support for per-region dynamically registered RPC endpoints for coprocessors and allow configurable RPC client/server implementations

I think it's tricky, because we dont really encourage people to think
of regions, but think of rows instead.  The fact that regions exist is
a bit of an implementation detail, although like indexes in databases
a critical and crucial one that we cant really ignore in the end.
Requiring people to specify regions by start/stop row but without
giving them a lot of support to discover and identify regions might
not be such a good idea.  Especially if we allow people to specify
arbitrary row ranges without regards to regions, and the rest is 'just
implementation'... it might be better to stick to something like a
pair of row keys or something.

On Thu, Sep 30, 2010 at 5:43 PM, Gary Helmling <ghelmling@...> wrote:
>
>
>> On 2010-09-25 16:18:25, Andrew Purtell wrote:
>> > src/main/java/org/apache/hadoop/hbase/client/HTable.java, line 1400
>> > <http://review.cloudera.org/r/816/diff/5/?file=12533#file12533line1400>
>> >
>> >     Maybe for sake of clarity call this getStartKeysInRange?
>>
>> Gary Helmling wrote:
>>     Makes sense as well, will rename.
>>
>> Himanshu Vashishtha wrote:
>>     Gary: I meant this use case (might be irrelevant, considering scope of cp):
>>     If I want to know count of number of rows in a range (Row A, Row B): with the help of this method, I can get
the starting row of all regions that are in this range but for the first (where you take the starting row of
the range: Row A in this case).
>>     So, when the processing is done in the last region, one shd be aware of the last row in the range, Row B.
This processing will be done in the cp impl, but that impl will be same on all region servers, so that check
(Continue reading)

Permalink | Reply | 1 comment |

Gmane