If you guys are thinking dependency injection, using Guice, constructor injection and final variables for dependencies has been a great formula for me on some stuff recently. I'm not a Spring expert but it was almost no work and felt a lot safer than xml-wired setter injection.
On Tue, Sep 14, 2010 at 1:51 AM, <st...@duboce.net> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/829/#review1176 > ----------------------------------------------------------- > > > > src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java > <http://review.cloudera.org/r/829/#comment4028> > > Didn't realize one of these lifecycle things in Guava but then there's a > million different versions of Lifecycle I suppose (I'm pretty sure I've > written a few myself in past life). We could write our own but, its my > guess it'd not differ from what we have here much. Then again its missing > Abort. > > This kinda refactoring is a big improvement; we should go this way, > yeah, as it takes us further down road already started where we're trying to > have Servers in hbase look alike. But I want us to be even more radical. I > want us to move up to something like Spring where implementing their > container means we get a bunch of other facility for near free and where the > wiring up of a regionserver or a master can be done from config with others > able to provide alternate implementations if they'd like with just a config. > change. It'd encourage writing to Interfaces, etc. ( Well, maybe not > Spring. Its too heavy weight and that xml stuff makes me queasy. There are > lesser IofC containers such as pico or nano). > > > > src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java > <http://review.cloudera.org/r/829/#comment4029> > > This is good > > > > src/main/java/org/apache/hadoop/hbase/master/HMaster.java > <http://review.cloudera.org/r/829/#comment4030> > > Thats a real pretty name (descriptive though I suppose) > > > > src/main/java/org/apache/hadoop/hbase/master/HMaster.java > <http://review.cloudera.org/r/829/#comment4031> > > We had a sort of convention for naming threads in hbase where host > 'server' is prefix where host includes service type and port if needed... > that kinda thing. Previous we had a naming convention per implementator... > was a mess. > > > > src/main/java/org/apache/hadoop/hbase/master/HMaster.java > <http://review.cloudera.org/r/829/#comment4032> > > I like having state in there. I was also adding zk sessionid.... > > > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > <http://review.cloudera.org/r/829/#comment4033> > > good > > > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > <http://review.cloudera.org/r/829/#comment4034> > > good > > > > > src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java > <http://review.cloudera.org/r/829/#comment4035> > > Good. I think I made this class originally. It grew into a monster. > > > > src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java > <http://review.cloudera.org/r/829/#comment4036> > > Are we losing the threaddumping? It was useful. > > > - stack > > > On 2010-09-13 18:36:29, Todd Lipcon wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://review.cloudera.org/r/829/ > > ----------------------------------------------------------- > > > > (Updated 2010-09-13 18:36:29) > > > > > > Review request for hbase, stack and Jonathan Gray. > > > > > > Summary > > ------- > > > > This isn't really cleaned up, but wanted to gather opinions before I put > in more effort. Does this kind of refactor seem good? Is the Guava "Service" > class what we want to use or should we just write our own similar interface? > > > > > > This addresses bug HBASE-2993. > > http://issues.apache.org/jira/browse/HBASE-2993 > > > > > > Diffs > > ----- > > > > src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java bc0a62f > > src/main/java/org/apache/hadoop/hbase/master/HMaster.java bc21a1e > > src/main/java/org/apache/hadoop/hbase/master/HMasterCommandLine.java > c675db9 > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > bba7b67 > > src/main/java/org/apache/hadoop/hbase/service/ServiceUtils.java > PRE-CREATION > > > src/main/java/org/apache/hadoop/hbase/service/ServiceWithMainThread.java > PRE-CREATION > > src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 4a9f1c3 > > src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java b7abb51 > > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 9cc1168 > > src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9407c1e > > src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java > 1cd88d3 > > src/test/java/org/apache/hadoop/hbase/TestInfoServers.java daffe02 > > src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java > 6908111 > > > src/test/java/org/apache/hadoop/hbase/master/TestKillingServersFromMaster.java > f5fd243 > > > src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java > a172e2c > > > src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java > 5b8b464 > > > src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java > 287f1fb > > > > Diff: http://review.cloudera.org/r/829/diff > > > > > > Testing > > ------- > > > > Tried to run unit tests, but plenty failed. I think they're failing on > trunk, too, though. Will do more testing and another round of review before > I actually claim this is ready to commit. > > > > > > Thanks, > > > > Todd > > > > > >
