Re: Review Request: HBASE-2578

2010-05-31 Thread Daniel Ploeg
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/ --- (Updated 2010-05-31 14:18:59.665128) Review request for hbase. Changes --- Cha

Re: Review Request: HBASE-2578

2010-05-27 Thread Daniel Ploeg
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/ --- (Updated 2010-05-27 19:16:50.127955) Review request for hbase. Changes --- Add

Re: Review Request: HBASE-2578

2010-05-27 Thread Ryan Rawson
Would you mind using mockito? I am trying to move us there and this would be a good first step. Thanks! On Thu, May 27, 2010 at 5:29 PM, Daniel Ploeg wrote: > > >> On 2010-05-27 16:45:23, Ryan Rawson wrote: >> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2701 >> >

Re: Review Request: HBASE-2578

2010-05-27 Thread Daniel Ploeg
> On 2010-05-27 16:45:23, Ryan Rawson wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2701 > > > > > > I'm thinking perhaps a Static interface could be used instead of the > > getDelegate().curre

Re: Review Request: HBASE-2578

2010-05-27 Thread Ryan Rawson
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/#review88 --- src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

Re: Review Request: HBASE-2578

2010-05-27 Thread Daniel Ploeg
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/ --- (Updated 2010-05-27 14:51:48.073426) Review request for hbase. Changes --- Usi

Re: Review Request: HBASE-2578

2010-05-26 Thread Paul Cowan
On 26/05/2010 4:26 PM, Ryan Rawson wrote: Sounds like we want #1: replaceable singleton. does that sound right? Yep. :) Hence volatile should be all you need in this case. It gets complicated if there's additional thread-safe logic needed in the getter or setter, but in this case that doesn

Re: Review Request: HBASE-2578

2010-05-25 Thread Ryan Rawson
Sounds like we want #1: replaceable singleton. does that sound right? On Tue, May 25, 2010 at 9:16 PM, Paul Cowan wrote: > On 26/05/2010 1:54 PM, Ryan Rawson wrote: >> >> Thanks for that salient comment. Perhaps someone can give us the right >> pattern for no lock Singleton initialization. > > T

Re: Review Request: HBASE-2578

2010-05-25 Thread Paul Cowan
On 26/05/2010 1:54 PM, Ryan Rawson wrote: Thanks for that salient comment. Perhaps someone can give us the right pattern for no lock Singleton initialization. There are really only two things which complicate thread-safe singletons: 1) Having a setter, so you can replace the singleton 2) Lazy

Re: Review Request: HBASE-2578

2010-05-25 Thread Todd Lipcon
Why not just initialize it from a static {} block, then in a test if we want to change it, we change it? On Tue, May 25, 2010 at 8:54 PM, Ryan Rawson wrote: > Thanks for that salient comment. Perhaps someone can give us the right > pattern for no lock Singleton initialization. > > On May 25, 201

Re: Review Request: HBASE-2578

2010-05-25 Thread Ryan Rawson
Thanks for that salient comment. Perhaps someone can give us the right pattern for no lock Singleton initialization. On May 25, 2010 8:49 PM, "Paul Cowan" wrote: > On 26/05/2010 1:15 PM, Daniel Ploeg wrote: this is a lot of synchronization for what ends up being on the fast path of every que

Re: Review Request: HBASE-2578

2010-05-25 Thread Paul Cowan
On 26/05/2010 1:15 PM, Daniel Ploeg wrote: this is a lot of synchronization for what ends up being on the fast path of every query, perhaps there is a non synchronized manner in which we can accomplish this? the use of volatile might help here thanks Ryan. I think I might use AtomicRefe

Re: Review Request: HBASE-2578

2010-05-25 Thread Daniel Ploeg
> On 2010-05-25 18:15:35, Ryan Rawson wrote: > > src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdgeManager.java, > > line 41 > > > > > > this is a lot of synchronization for what ends up being on the fast > > path of ever

Re: Review Request: HBASE-2578

2010-05-25 Thread Ryan Rawson
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/#review72 --- src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdgeManager.java

Re: Review Request: HBASE-2578

2010-05-25 Thread Daniel Ploeg
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/ --- (Updated 2010-05-25 17:49:37.154103) Review request for hbase. Changes --- Bas

Re: Review Request: HBASE-2578

2010-05-25 Thread stack
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/#review65 --- Ship it! Implementation is well done. The need for this though strikes me a

Re: Review Request: HBASE-2578

2010-05-25 Thread Ryan Rawson
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/#review62 --- What other possible approaches could we take to solving this issue? I seem t

Re: Review Request: HBASE-2578

2010-05-25 Thread Daniel Ploeg
> On 2010-05-25 12:39:39, Todd Lipcon wrote: > > src/main/java/org/apache/hadoop/hbase/util/IncrementingEnvironmentEdge.java, > > line 17 > > > > > > may need to be synchronized, or use AtomicLong Thanks Todd. - I'm modifying t

Re: Review Request: HBASE-2578

2010-05-25 Thread Todd Lipcon
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/ --- (Updated 2010-05-25 12:40:01.904248) Review request for hbase. Changes --- Add

Re: Review Request: HBASE-2578

2010-05-25 Thread Todd Lipcon
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/#review56 --- Looks good. Few notes: - Please add the Apache license header to all the new

Review Request: HBASE-2578

2010-05-25 Thread Daniel Ploeg
--- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/ --- Review request for hbase. Summary --- HBASE-2578 - Add ability for tests to over