> On 2010-06-11 12:45:29, stack wrote: > > bin/replication/add_peer.rb, line 21 > > <http://review.hbase.org/r/76/diff/5/?file=1104#file1104line21> > > > > Should you point at some replication documentation here? Is there such > > a thing?
package.html later, should I point to it? > On 2010-06-11 12:45:29, stack wrote: > > bin/replication/copy_tables_desc.rb, line 58 > > <http://review.hbase.org/r/76/diff/5/?file=1105#file1105line58> > > > > This could get a bit annoying I'd say. It helped me a lot, remove if people complain? > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/HConstants.java, line 342 > > <http://review.hbase.org/r/76/diff/5/?file=1107#file1107line342> > > > > This has to go here? Can it go into one of the replication classes? Used by master and region server, to me it belongs there. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/master/ServerManager.java, line 156 > > <http://review.hbase.org/r/76/diff/5/?file=1109#file1109line156> > > > > Can't you just do c.get("key", defaultvalue)? No, I also do a check on replication. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line > > 929 > > <http://review.hbase.org/r/76/diff/5/?file=1110#file1110line929> > > > > You writing startcode into zk? Why not write servername -- the > > host+port+startcode combo? To be coherent with the rest of the code that uses zookeeper. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line > > 1075 > > <http://review.hbase.org/r/76/diff/5/?file=1110#file1110line1075> > > > > Is this directory name? Confusingly named given rootdir+regLogPathStr > > only adds up to repLogPath. I don't understand you, but this code is going to be removed in my next patch as I'm simplifying RepSink. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 55 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line55> > > > > Peers are named '1', '2'? Can't we have more meaningful names here? We agreed that peers are identified with a short internally as it is stored. We could use an external mapping of short->cute_name. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 59 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line59> > > > > Use servername instead of startcode Same comment as before, needs to be coherent. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 60 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line60> > > > > All RS's in a master cluster replicate? Yep... was that an implicit way of saying that I need to document that in RZH? > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 107 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line107> > > > > Should this class be called WRapper instaad of Helper? Sure > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 185 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line185> > > > > You mean 'ensemble' here rather than 'quorum' (Patrick will kill you if > > he sees you calling it a 'quorum' when you mean the other) Argh I'm trying to correct myself but I'm still missing some of them. Thx! > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 263 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line263> > > > > We keep up the replication position in zk? How much do we replicate in > > one go? Its not a single edit, is it? We do this for every log file? Yes. A defined amount specified in ReplicationSource. No. Every current log file, we only replicate one at a time per region server. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 328 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line328> > > > > LOG.warn instead? > > I'll do like the rest and log.error > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 354 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line354> > > > > We return empty map if clusters size is == 1? Should that be > > clusters.size == 0? That part isn't clear enough, so the reason it's 1 and not 0 is that we put a lock in there so it's listed in the znodes we fetch. Actually this should be <= 1 rather than ==. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 356 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line356> > > > > Whats this about? See previous comment, we lock the dead region server's znode by putting a lock in there, but we don't want to process the hlogs under since... it's not a cluster. Could use more doc. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java, > > line 402 > > <http://review.hbase.org/r/76/diff/5/?file=1113#file1113line402> > > > > Just logging errors? What if session expired (our discussion from last > > day)? Yes I need to review how I handle it in RZH, but I'd also need to review ZKW since some methods will hid it in there. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/package.html, line 41 > > <http://review.hbase.org/r/76/diff/5/?file=1115#file1115line41> > > > > Call it alpha yeah! (j/k) > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/package.html, line 64 > > <http://review.hbase.org/r/76/diff/5/?file=1115#file1115line64> > > > > Whats this about? You need to run zk yourself but no zoo.cfg? I... don't remember why I wrote this. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/package.html, line 73 > > <http://review.hbase.org/r/76/diff/5/?file=1115#file1115line73> > > > > And if not? What if replicating single-family only? Forgot to update that after we added scoping, updating. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/package.html, line 83 > > <http://review.hbase.org/r/76/diff/5/?file=1115#file1115line83> > > > > Has to be offline? Will this always be the case? Currently everything is static, but I hope we can move on from that in the future. > On 2010-06-11 12:45:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/replication/package.html, line 108 > > <http://review.hbase.org/r/76/diff/5/?file=1115#file1115line108> > > > > whats ratio? This is a log snippet that's coming from a region server. Do you want to see more documentation about it in package.html or in the logging itself? - Jean-Daniel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/76/#review191 ----------------------------------------------------------- On 2010-06-08 17:54:19, Jean-Daniel Cryans wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/76/ > ----------------------------------------------------------- > > (Updated 2010-06-08 17:54:19) > > > Review request for hbase. > > > Summary > ------- > > This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview > patch" as it's pretty much feature complete, works on a cluster, has unit > tests and whatnot, but it could use a lot more testing and cleaning and ideas > from others. > > > This addresses bug HBASE-2223. > http://issues.apache.org/jira/browse/HBASE-2223 > > > Diffs > ----- > > bin/replication/add_peer.rb PRE-CREATION > bin/replication/copy_tables_desc.rb PRE-CREATION > pom.xml 03c6ec8 > src/main/java/org/apache/hadoop/hbase/HConstants.java 13aff26 > src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java b36f1df > src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 82148a6 > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > a1baff4 > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 034690e > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 5d4cffe > > src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java > PRE-CREATION > > src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java > PRE-CREATION > src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION > > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java > PRE-CREATION > > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java > PRE-CREATION > > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java > PRE-CREATION > > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 2f2f306 > > src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java > PRE-CREATION > > src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java > PRE-CREATION > > src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java > PRE-CREATION > > src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java > PRE-CREATION > > Diff: http://review.hbase.org/r/76/diff > > > Testing > ------- > > > Thanks, > > Jean-Daniel > >
