Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.92.0
    • Fix Version/s: 0.92.0
    • Component/s: Replication
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      We need to support cyclic replication by using the cluster id of each HlogKey and stop replicating when it goes back to the original cluster.

      1. 2195.txt
        20 kB
        Lars Hofhansl
      2. 2195-v5.txt
        40 kB
        Lars Hofhansl
      3. 2195-v6.txt
        40 kB
        Lars Hofhansl
      4. 2195-v10.txt
        52 kB
        Ted Yu
      5. 2195-v12.txt
        53 kB
        Ted Yu
      6. 2195-v13.txt
        53 kB
        Ted Yu
      7. 2195-v14.txt
        53 kB
        Ted Yu

        Activity

        Hide
        stack added a comment -

        This is going to be done for the 0.90.0 timeframe? If not, lets move it out.

        Show
        stack added a comment - This is going to be done for the 0.90.0 timeframe? If not, lets move it out.
        Hide
        stack added a comment -

        This going to be done for 0.90? If not, can we move it out?

        Show
        stack added a comment - This going to be done for 0.90? If not, can we move it out?
        Hide
        Jean-Daniel Cryans added a comment -

        Moving out to 0.92.0 for the moment.

        Show
        Jean-Daniel Cryans added a comment - Moving out to 0.92.0 for the moment.
        Hide
        stack added a comment -

        Moving out of 0.92.0. Pull it back in if you think different.

        Show
        stack added a comment - Moving out of 0.92.0. Pull it back in if you think different.
        Hide
        Lars Hofhansl added a comment -

        There is a good chance that we will need this at Salesforce.com.

        After looking at the code, the only missing piece is retaining the originating cluster information at the ReplicationSink. (Unless, of course, I am missing something).

        I have some test code hacked up, which allows setting the cluster id as a Put/Delete attribute and then in HRegion writes the ClusterId into the WAL.
        The replication source checks the HLog key and does not replicate when the cluster id equals the peer's cluster id, in addition if the cluster id was set in the WAL it is retained.

        The basic problem is that this requires core changes (HRegion), and replication so far is nicely out of core.

        Show
        Lars Hofhansl added a comment - There is a good chance that we will need this at Salesforce.com. After looking at the code, the only missing piece is retaining the originating cluster information at the ReplicationSink. (Unless, of course, I am missing something). I have some test code hacked up, which allows setting the cluster id as a Put/Delete attribute and then in HRegion writes the ClusterId into the WAL. The replication source checks the HLog key and does not replicate when the cluster id equals the peer's cluster id, in addition if the cluster id was set in the WAL it is retained. The basic problem is that this requires core changes (HRegion), and replication so far is nicely out of core.
        Hide
        Jean-Daniel Cryans added a comment -

        Yep that's pretty much it. ICVs should be replicated too.

        Show
        Jean-Daniel Cryans added a comment - Yep that's pretty much it. ICVs should be replicated too.
        Hide
        Lars Hofhansl added a comment -

        Thanks...
        So ICVs currently are not replicated, correct? (or maybe they are translated into puts at the ReplicationSink?)

        Show
        Lars Hofhansl added a comment - Thanks... So ICVs currently are not replicated, correct? (or maybe they are translated into puts at the ReplicationSink?)
        Hide
        Jean-Daniel Cryans added a comment -

        Translated into puts containing the new value (not a delta).

        Show
        Jean-Daniel Cryans added a comment - Translated into puts containing the new value (not a delta).
        Hide
        Lars Hofhansl added a comment -

        I have a patch working.

        Part of this is splicing the cluster id into the Puts and Deletes generated at the Slave (ReplicationSink) and then writing it to the WAL on the Slave.
        Currently I am using

        {get|set}

        Attribute(...) on both Puts and Deletes. Then in HRegion I check for these attributes and write the cluster id to the log.

        What the general thought here? Do this? Or increment the versions of the Put and Delete classes and add a new byte member for the cluster id? Or something entirely different that does not involve core (HRegion) needing to know about cluster ids now?

        Show
        Lars Hofhansl added a comment - I have a patch working. Part of this is splicing the cluster id into the Puts and Deletes generated at the Slave (ReplicationSink) and then writing it to the WAL on the Slave. Currently I am using {get|set} Attribute(...) on both Puts and Deletes. Then in HRegion I check for these attributes and write the cluster id to the log. What the general thought here? Do this? Or increment the versions of the Put and Delete classes and add a new byte member for the cluster id? Or something entirely different that does not involve core (HRegion) needing to know about cluster ids now?
        Hide
        Ted Yu added a comment -

        I don't think a byte would be able to represent cluster Id. From ClusterId.java:

          private String id;
        

        Looks like your current approach is more viable.

        Show
        Ted Yu added a comment - I don't think a byte would be able to represent cluster Id. From ClusterId.java: private String id; Looks like your current approach is more viable.
        Hide
        stack added a comment -

        Adding clusterid to the attribute map seems fine though it probably be more efficient having clusterid a native data member on Put and Delete.

        Current ClusterId is UUID (See MasterFileSystem where its written). It takes 128bits to represent uuid in bits only. Is that too much per edit to carry replicating? It'd for sure be 'safe' such that cluster ids would never 'clash' because someone messed up the mappings of clusterid to some short form.

        Show
        stack added a comment - Adding clusterid to the attribute map seems fine though it probably be more efficient having clusterid a native data member on Put and Delete. Current ClusterId is UUID (See MasterFileSystem where its written). It takes 128bits to represent uuid in bits only. Is that too much per edit to carry replicating? It'd for sure be 'safe' such that cluster ids would never 'clash' because someone messed up the mappings of clusterid to some short form.
        Hide
        Andrew Purtell added a comment -

        It takes 128bits to represent uuid in bits only. Is that too much per edit to carry replicating?

        Better to group edits and provide the UUID once in the wrapper perhaps? ReplEdit, like WALEdit? At least would cut down from 128 bits every edit always.

        It'd for sure be 'safe' such that cluster ids would never 'clash' because someone messed up the mappings of clusterid to some short form.

        It is reasonable to prefer safety, especially if we are going to be supporting arbitrary (or at least flexible) replication topologies as a feature.

        Show
        Andrew Purtell added a comment - It takes 128bits to represent uuid in bits only. Is that too much per edit to carry replicating? Better to group edits and provide the UUID once in the wrapper perhaps? ReplEdit, like WALEdit? At least would cut down from 128 bits every edit always. It'd for sure be 'safe' such that cluster ids would never 'clash' because someone messed up the mappings of clusterid to some short form. It is reasonable to prefer safety, especially if we are going to be supporting arbitrary (or at least flexible) replication topologies as a feature.
        Hide
        Lars Hofhansl added a comment -

        The ClusterId as byte part actually comes from the replication code.
        ( See ReplicationSource.clusterId and HLogKey.

        {get|set}

        ClusterId(...) )

        You "label" the peer cluster when you add it with ReplicationAdmin.addPeer. And the "ClusterId" here is just the ZK identifier for the cluster for replication purposes.
        It just means we never have a replication cycle > 256 clusters, which seems OK.

        Since this ends up in every log entry, it should be kept small. And since HLogKey already has this byte, it wouldn't be a format change either.

        Maybe J-D can chime in?

        Show
        Lars Hofhansl added a comment - The ClusterId as byte part actually comes from the replication code. ( See ReplicationSource.clusterId and HLogKey. {get|set} ClusterId(...) ) You "label" the peer cluster when you add it with ReplicationAdmin.addPeer. And the "ClusterId" here is just the ZK identifier for the cluster for replication purposes. It just means we never have a replication cycle > 256 clusters, which seems OK. Since this ends up in every log entry, it should be kept small. And since HLogKey already has this byte, it wouldn't be a format change either. Maybe J-D can chime in?
        Hide
        Lars Hofhansl added a comment -

        (Comment overlap.)

        Grouping edits is good idea.
        The originating cluster will always just have WALEdits (unless we write WALEdits and ReplEdits).
        At the first replicated cluster we'd then write ReplEdits, which also have to function as WALEdits (unless we write both again).

        At the same time I like the simplicity of WAL shipping and just creating Puts and Deletes at the replication site to deploy the changes.

        Show
        Lars Hofhansl added a comment - (Comment overlap.) Grouping edits is good idea. The originating cluster will always just have WALEdits (unless we write WALEdits and ReplEdits). At the first replicated cluster we'd then write ReplEdits, which also have to function as WALEdits (unless we write both again). At the same time I like the simplicity of WAL shipping and just creating Puts and Deletes at the replication site to deploy the changes.
        Hide
        Jean-Daniel Cryans added a comment -

        Better to group edits and provide the UUID once in the wrapper perhaps? ReplEdit, like WALEdit? At least would cut down from 128 bits every edit always.

        Grouping edits is good idea.

        Right now HLogKey has the byte that identifies the cluster, and there's only one per WALEdit. Meaning it's already grouped.

        What the general thought here? Do this? Or increment the versions of the Put and Delete classes and add a new byte member for the cluster id?

        One one hand, if it's an attribute it means there's a possibility we can ask the cluster if it supports replication and in the case it doesn't then you don't have to add it. On the other hand, it's more complex.

        You could also add it in MultiAction so that grouped Puts and Deletes wouldn't have to carry the same information N times.

        Show
        Jean-Daniel Cryans added a comment - Better to group edits and provide the UUID once in the wrapper perhaps? ReplEdit, like WALEdit? At least would cut down from 128 bits every edit always. Grouping edits is good idea. Right now HLogKey has the byte that identifies the cluster, and there's only one per WALEdit. Meaning it's already grouped. What the general thought here? Do this? Or increment the versions of the Put and Delete classes and add a new byte member for the cluster id? One one hand, if it's an attribute it means there's a possibility we can ask the cluster if it supports replication and in the case it doesn't then you don't have to add it. On the other hand, it's more complex. You could also add it in MultiAction so that grouped Puts and Deletes wouldn't have to carry the same information N times.
        Hide
        Lars Hofhansl added a comment -

        Thanks J-D.
        To make the discussion a bit more concrete I am attaching a sketch of a patch.
        This is hack for now, but it shows the general direction.
        In my limited testing it worked fine.

        Show
        Lars Hofhansl added a comment - Thanks J-D. To make the discussion a bit more concrete I am attaching a sketch of a patch. This is hack for now, but it shows the general direction. In my limited testing it worked fine.
        Hide
        Lars Hofhansl added a comment -

        And I should note that this is the absolute minimum to make it work (I only changed the Put/Delete paths that are actually used by replication).

        Show
        Lars Hofhansl added a comment - And I should note that this is the absolute minimum to make it work (I only changed the Put/Delete paths that are actually used by replication).
        Hide
        Jean-Daniel Cryans added a comment -

        Looks fine to me.

        Show
        Jean-Daniel Cryans added a comment - Looks fine to me.
        Hide
        Lars Hofhansl added a comment -

        We'll be testing this internally over the next few days.

        Show
        Lars Hofhansl added a comment - We'll be testing this internally over the next few days.
        Hide
        stack added a comment -

        So, we'd go with using the attribute and the attribute would have the uuid for the cluster. It'd be set on every edit rather than per every HLogEdit (which could have one or a batch of edits). Chatting w/ J-D, he reminds me that there is not yet a means of correlating the byte-level cluster identifier with the uuid cluster id we have for CPs (there is no cluster registry where the byte gets mapped to uuid).

        Downsides:
        + Going this route would make the byte id redundant; might as well clear it out if we are not going to use it
        + Fat UUID repeated per individual edit (in a Map so fatter still, rather than as data member bits in byte array)

        Upsides:
        + 'safe' – no need of cluster registry
        + Minimal changes required to core types getting cyclic replication working

        Whats your thought Lars... get this attribute based version working then optimize space-used, etc., later?

        Show
        stack added a comment - So, we'd go with using the attribute and the attribute would have the uuid for the cluster. It'd be set on every edit rather than per every HLogEdit (which could have one or a batch of edits). Chatting w/ J-D, he reminds me that there is not yet a means of correlating the byte-level cluster identifier with the uuid cluster id we have for CPs (there is no cluster registry where the byte gets mapped to uuid). Downsides: + Going this route would make the byte id redundant; might as well clear it out if we are not going to use it + Fat UUID repeated per individual edit (in a Map so fatter still, rather than as data member bits in byte array) Upsides: + 'safe' – no need of cluster registry + Minimal changes required to core types getting cyclic replication working Whats your thought Lars... get this attribute based version working then optimize space-used, etc., later?
        Hide
        Lars Hofhansl added a comment -

        I have been thinking about this too. Actually I found that the cluster id in ReplicationSource is always 0 (i.e. it is never set for the Master - and my tests just worked by accident ).

        In fact I came to the same conclusion...
        I think the main reason that it is nice that the replication members do not need to be labeled. Knowing their zookeeper ensemble allows us to get the UUID.

        I have a patch that uses the UUID about 70% done.
        One of the problems is that now one cannot enable replication without the other side running as it needs to connect to the other zookeeper to get the cluster id (but I can probably work around that by retrieving the UUID later).

        I think getting the attribute based version running is a good start. Then we can decide whether this is safe enough for 0.92 or we'll do a space optimized version for 0.94.

        Show
        Lars Hofhansl added a comment - I have been thinking about this too. Actually I found that the cluster id in ReplicationSource is always 0 (i.e. it is never set for the Master - and my tests just worked by accident ). In fact I came to the same conclusion... I think the main reason that it is nice that the replication members do not need to be labeled. Knowing their zookeeper ensemble allows us to get the UUID. I have a patch that uses the UUID about 70% done. One of the problems is that now one cannot enable replication without the other side running as it needs to connect to the other zookeeper to get the cluster id (but I can probably work around that by retrieving the UUID later). I think getting the attribute based version running is a good start. Then we can decide whether this is safe enough for 0.92 or we'll do a space optimized version for 0.94.
        Hide
        stack added a comment -

        +1 on the above.

        Show
        stack added a comment - +1 on the above.
        Hide
        Lars Hofhansl added a comment -

        Now I have to wrap my head around the naming convention in Zookeeper.
        Currently this is all labeled with the 1 digit cluster id.
        '-' is used as separator in case of ZK failures.
        So the ClusterUUID needs to be encoded (maybe as simple as replacing - with x). Base64 encoding is no good because it contains / . Or maybe it is simpler to use ; as separator now.

        I am not aware of any length limits for ZNode names, is that correct?

        Is the description here http://hbase.apache.org/replication.html, still completely up-to-date?

        Show
        Lars Hofhansl added a comment - Now I have to wrap my head around the naming convention in Zookeeper. Currently this is all labeled with the 1 digit cluster id. '-' is used as separator in case of ZK failures. So the ClusterUUID needs to be encoded (maybe as simple as replacing - with x). Base64 encoding is no good because it contains / . Or maybe it is simpler to use ; as separator now. I am not aware of any length limits for ZNode names, is that correct? Is the description here http://hbase.apache.org/replication.html , still completely up-to-date?
        Hide
        Lars Hofhansl added a comment -

        Ah one more. Can I just how change HLogKey is read or written? It seems that this was done before (when byte cluster id was added), but I don't see how that would just work - it only guards again EOFException in readFields.

        Show
        Lars Hofhansl added a comment - Ah one more. Can I just how change HLogKey is read or written? It seems that this was done before (when byte cluster id was added), but I don't see how that would just work - it only guards again EOFException in readFields.
        Hide
        Lars Hofhansl added a comment -

        Amending my solution a bit. The Edits will be written with the full id as described above.
        In the near term I think it is better keep the 1 character ID for local identification.

        The reason is:
        1. It is nice to be able to identify every slave by a number chosen by the master. (addPeer, deletePeer, listPeers, etc) - once we support multiple slaves that is.
        2. A user at the master does not have to know about cluster UUID.
        3. The ZNode handling would not change.

        These id is never shared to the slave/sink.
        Under the hood the full 16byte cluster id is used in the Edits to avoid replication cycles.

        I assume it would be worth going through the trouble of not storing those 16 bytes of UUID when replication is not enabled

        Show
        Lars Hofhansl added a comment - Amending my solution a bit. The Edits will be written with the full id as described above. In the near term I think it is better keep the 1 character ID for local identification. The reason is: 1. It is nice to be able to identify every slave by a number chosen by the master. (addPeer, deletePeer, listPeers, etc) - once we support multiple slaves that is. 2. A user at the master does not have to know about cluster UUID. 3. The ZNode handling would not change. These id is never shared to the slave/sink. Under the hood the full 16byte cluster id is used in the Edits to avoid replication cycles. I assume it would be worth going through the trouble of not storing those 16 bytes of UUID when replication is not enabled
        Hide
        stack added a comment -

        So the ClusterUUID needs to be encoded (maybe as simple as replacing - with x). Base64 encoding is no good because it contains / . Or maybe it is simpler to use ; as separator now.

        Thats unfortunate. Yeah, replace the hyphen with an underscore or something (the UUID toString is no help here).

        I am not aware of any length limits for ZNode names, is that correct?

        Can you find any in zk apis? I don't know if there are limits. If you can't figure it, we can ask the lads.

        on http://hbase.apache.org/replication.html

        Bug J-D on Tuesday after he gets back from holidays. He did a presentation on it recently; that might be more up-to-date than whats on website.. he'd know best.

        Show
        stack added a comment - So the ClusterUUID needs to be encoded (maybe as simple as replacing - with x). Base64 encoding is no good because it contains / . Or maybe it is simpler to use ; as separator now. Thats unfortunate. Yeah, replace the hyphen with an underscore or something (the UUID toString is no help here). I am not aware of any length limits for ZNode names, is that correct? Can you find any in zk apis? I don't know if there are limits. If you can't figure it, we can ask the lads. on http://hbase.apache.org/replication.html Bug J-D on Tuesday after he gets back from holidays. He did a presentation on it recently; that might be more up-to-date than whats on website.. he'd know best.
        Hide
        stack added a comment -

        Can I just how change HLogKey is read or written? It seems that this was done before (when byte cluster id was added), but I don't see how that would just work - it only guards again EOFException in readFields.

        Yes. You can. While you are at it, can you make it versioned? Have it subclass this, http://hadoop.apache.org/common/docs/current/api/org/apache/hadoop/io/VersionedWritable.html, instead of just Writable.

        This change will be included in 0.92, right? Our next major version. We need to state this in stone somewhere but just as (currently), rpc versioning makes it so old versions of hbase can not talk across a major version upgrade, similar, we can say that new major version cannot read old version WAL logs.

        If you have HLogKey subclass VersionedWritable, going forward, we'll have some of evolving the HLogKey in a manner in which new HLogKey versions will be able to identify and deserialize old objects into new.

        I assume it would be worth going through the trouble of not storing those 16 bytes of UUID when replication is not enabled

        Yeah, if you can.

        So, the patch will not include being able to map from single byte cluserid to uuid clusterid? That'd be done elsewhere?

        Show
        stack added a comment - Can I just how change HLogKey is read or written? It seems that this was done before (when byte cluster id was added), but I don't see how that would just work - it only guards again EOFException in readFields. Yes. You can. While you are at it, can you make it versioned? Have it subclass this, http://hadoop.apache.org/common/docs/current/api/org/apache/hadoop/io/VersionedWritable.html , instead of just Writable. This change will be included in 0.92, right? Our next major version. We need to state this in stone somewhere but just as (currently), rpc versioning makes it so old versions of hbase can not talk across a major version upgrade, similar, we can say that new major version cannot read old version WAL logs. If you have HLogKey subclass VersionedWritable, going forward, we'll have some of evolving the HLogKey in a manner in which new HLogKey versions will be able to identify and deserialize old objects into new. I assume it would be worth going through the trouble of not storing those 16 bytes of UUID when replication is not enabled Yeah, if you can. So, the patch will not include being able to map from single byte cluserid to uuid clusterid? That'd be done elsewhere?
        Hide
        Lars Hofhansl added a comment -

        OK... Right now I made it versioned by observing the current byte cluster id is always 0, so I was using this as version byte. I'll look into VersionedWritable.

        I also have conditioned code that only writes the 16bytes when needed (at the expense of having to write an extra boolean).

        re: Byte ids... The bytes id is just used locally at a replication source. It has no meaning elsewhere and never ends in the WAL. Otherwise we have to rethink how one adds and removes sinks at a source (i.e. how do you identify which sink to remove, unless you like to type in 16 byte ids). Changing this would also mean that we cannot setup the ZNodes at a replication before the sink is up - as we cannot get the sinks UUID before it can be reached.
        This is actually a fairly clean design (IMHO), as a user at a source can give the sinks numbers. And these numbers will never clash with the same numbers at another source.

        I'll attach a patch later today, which hopefully makes it clear what I have in mind.

        Show
        Lars Hofhansl added a comment - OK... Right now I made it versioned by observing the current byte cluster id is always 0, so I was using this as version byte. I'll look into VersionedWritable. I also have conditioned code that only writes the 16bytes when needed (at the expense of having to write an extra boolean). re: Byte ids... The bytes id is just used locally at a replication source. It has no meaning elsewhere and never ends in the WAL. Otherwise we have to rethink how one adds and removes sinks at a source (i.e. how do you identify which sink to remove, unless you like to type in 16 byte ids). Changing this would also mean that we cannot setup the ZNodes at a replication before the sink is up - as we cannot get the sinks UUID before it can be reached. This is actually a fairly clean design (IMHO), as a user at a source can give the sinks numbers. And these numbers will never clash with the same numbers at another source. I'll attach a patch later today, which hopefully makes it clear what I have in mind.
        Hide
        Lars Hofhansl added a comment -

        New version of the patch.
        As you can see the local id (set with addPeer) is only for local bookkeeping in the source cluster.
        Behind the scenes the WAL keeps track of the cluster's UUID, but that is all transparent to a user setting up things with addpeer.

        Show
        Lars Hofhansl added a comment - New version of the patch. As you can see the local id (set with addPeer) is only for local bookkeeping in the source cluster. Behind the scenes the WAL keeps track of the cluster's UUID, but that is all transparent to a user setting up things with addpeer.
        Hide
        Ted Yu added a comment -

        In HLogKey.readFields():

        +      if (version < 0) {
        

        I think the above check may not be safe in case of an old key which didn't use byte clusterId.
        If backward compatibility is not maintained as Stack indicated, version should be the first byte read in readFields().

        Show
        Ted Yu added a comment - In HLogKey.readFields(): + if (version < 0) { I think the above check may not be safe in case of an old key which didn't use byte clusterId. If backward compatibility is not maintained as Stack indicated, version should be the first byte read in readFields().
        Hide
        Lars Hofhansl added a comment -

        True true. Those would be very old logs from before replication was added, though.

        If we break compatibility, how are we going to make sure that there no old logs to read when a user upgrades to 0.92? In this case we will guarantee that old logs cannot be read any longer.
        In addition there would be no way to check whether a log is old, as the first byte can be anything.

        Show
        Lars Hofhansl added a comment - True true. Those would be very old logs from before replication was added, though. If we break compatibility, how are we going to make sure that there no old logs to read when a user upgrades to 0.92? In this case we will guarantee that old logs cannot be read any longer. In addition there would be no way to check whether a log is old, as the first byte can be anything.
        Hide
        Lars Hofhansl added a comment -

        Actually even the current code is not safe against an old key (before cluster id was introduced).
        See HLogKey.readFields(), see only guard is catching EOFE, which will only be triggered at the end of a file.

        Show
        Lars Hofhansl added a comment - Actually even the current code is not safe against an old key (before cluster id was introduced). See HLogKey.readFields(), see only guard is catching EOFE, which will only be triggered at the end of a file.
        Hide
        Ted Yu added a comment -

        I don't see HLogKey extend VersionedWritable in patch version 5.

        Show
        Ted Yu added a comment - I don't see HLogKey extend VersionedWritable in patch version 5.
        Hide
        Ted Yu added a comment - - edited

        If we look at Bytes.readByteArray():

          public static byte [] readByteArray(final DataInput in)
          throws IOException {
            int len = WritableUtils.readVInt(in);
            if (len < 0) {
              throw new NegativeArraySizeException(Integer.toString(len));
            }
        

        we can see that if the new version field is declared as int, we should be able to serialize it as VInt.
        What remains is to customize Bytes.readByteArray() a little bit to fit the needs for this JIRA.

        Long term, I think the mapping from single byte cluserid to uuid clusterid should be designed/implemented first. This aligns with Lars' effort of writing the UUID conditionally.

        Show
        Ted Yu added a comment - - edited If we look at Bytes.readByteArray(): public static byte [] readByteArray(final DataInput in) throws IOException { int len = WritableUtils.readVInt(in); if (len < 0) { throw new NegativeArraySizeException(Integer.toString(len)); } we can see that if the new version field is declared as int, we should be able to serialize it as VInt. What remains is to customize Bytes.readByteArray() a little bit to fit the needs for this JIRA. Long term, I think the mapping from single byte cluserid to uuid clusterid should be designed/implemented first. This aligns with Lars' effort of writing the UUID conditionally.
        Hide
        Lars Hofhansl added a comment -

        @Ted
        re HLogKey: Correct, it does not extend VersionWritable. This way I can make sure that 0.92 can read all log files that 0.90.x could read (currently 0.90.x might fail on earlier log files, but that damage is already done)
        If we're not concerned about this, I agree using VersionedWritable is better.

        re byte cluster id to UUID: What I am saying is that we do not need a mapping. A replication source used some random numbering scheme (that we can change at any given time). When the log is replicated it is tagged the correct UUID. And as an optimization a source also retrieves the sink's cluster UUID via ZK to avoid shipping logs if not needed.
        With some restructuring (that I don't think should go into 0.92), the byte cluster id can just go away. That involves backwards compatibility needs in how to read existing ZNode entries and rethinking how a replication source would maintain its sink(s).

        re readByteArray: I think I am missing something. As the old logs can have any byte combination at the beginning, we'll always potentially run into problems. So either we break compatibility
        once to start using VersionedWritable now, or use the former cluster id byte going forward and that would provide backwards compatibility in 0.92.

        Show
        Lars Hofhansl added a comment - @Ted re HLogKey: Correct, it does not extend VersionWritable. This way I can make sure that 0.92 can read all log files that 0.90.x could read (currently 0.90.x might fail on earlier log files, but that damage is already done) If we're not concerned about this, I agree using VersionedWritable is better. re byte cluster id to UUID: What I am saying is that we do not need a mapping . A replication source used some random numbering scheme (that we can change at any given time). When the log is replicated it is tagged the correct UUID. And as an optimization a source also retrieves the sink's cluster UUID via ZK to avoid shipping logs if not needed. With some restructuring (that I don't think should go into 0.92), the byte cluster id can just go away. That involves backwards compatibility needs in how to read existing ZNode entries and rethinking how a replication source would maintain its sink(s). re readByteArray: I think I am missing something. As the old logs can have any byte combination at the beginning, we'll always potentially run into problems. So either we break compatibility once to start using VersionedWritable now, or use the former cluster id byte going forward and that would provide backwards compatibility in 0.92.
        Hide
        Ted Yu added a comment - - edited

        Below is what I was thinking in terms of reading version:

          /**
           * Read WritableableUtils.vint prefix and see if it represents version
           *   which should be less than 0
           * if no version is found, treat the prefix as the length for the byte-array
           * @param in Input to read from.
           * @throws IOException e
           */
          public void readVersinAndRegionName(final DataInput in)
          throws IOException {
            this.version = 0;
            int len = 0;
            int lenOrVer = WritableUtils.readVInt(in);
            if (lenOrVer < 0) {   // we found the version
              this.version = (byte)(lenOrVer & 0xFF);
              len = WritableUtils.readVInt(in);
            } else {
              len = lenOrVer;
            }
            this.encodedRegionName = new byte[len];
            in.readFully(this.encodedRegionName, 0, len);
          }
        
        Show
        Ted Yu added a comment - - edited Below is what I was thinking in terms of reading version: /** * Read WritableableUtils.vint prefix and see if it represents version * which should be less than 0 * if no version is found, treat the prefix as the length for the byte-array * @param in Input to read from. * @throws IOException e */ public void readVersinAndRegionName(final DataInput in) throws IOException { this.version = 0; int len = 0; int lenOrVer = WritableUtils.readVInt(in); if (lenOrVer < 0) { // we found the version this.version = (byte)(lenOrVer & 0xFF); len = WritableUtils.readVInt(in); } else { len = lenOrVer; } this.encodedRegionName = new byte[len]; in.readFully(this.encodedRegionName, 0, len); }
        Hide
        Ted Yu added a comment -

        w.r.t. VersionedWritable, we just need to add getVersion() which would return the least significant byte of version.
        It would take some time before we consume all the 128 values of version byte.

        Show
        Ted Yu added a comment - w.r.t. VersionedWritable, we just need to add getVersion() which would return the least significant byte of version. It would take some time before we consume all the 128 values of version byte.
        Hide
        Lars Hofhansl added a comment -

        Oh I see now. Clever.
        Not safer than using the cluster id byte, though, and more complicated

        Show
        Lars Hofhansl added a comment - Oh I see now. Clever. Not safer than using the cluster id byte, though, and more complicated
        Hide
        Ted Yu added a comment -

        It is a little complicated.
        I think it is quite safe - encodedRegionName should have been there for any HLogKey.

        Show
        Ted Yu added a comment - It is a little complicated. I think it is quite safe - encodedRegionName should have been there for any HLogKey.
        Hide
        Lars Hofhansl added a comment -

        Same patch with Ted's suggestion (slightly modified)

        Show
        Lars Hofhansl added a comment - Same patch with Ted's suggestion (slightly modified)
        Hide
        Ted Yu added a comment -

        Patch v6 looks better. Some javadoc is missing (e.g. readFields)
        It should be easy to add getVersion() for HLogKey.

        Show
        Ted Yu added a comment - Patch v6 looks better. Some javadoc is missing (e.g. readFields) It should be easy to add getVersion() for HLogKey.
        Hide
        Lars Hofhansl added a comment -

        ReadFields is documented at the interface level (whether it is internally versioned should be of no interest to callers).

        Subclassing VersionedWritable has no advantage here (we can't use super.readFields nor super.write), so I'd rather not leak the versioning stuff out of HLogKey via a getVersion() method until we need it.

        My main concern is w.r.t. the changes to HRegion, because that now has the cluster UUID sprinkled all over.

        All tests pass now, and I did some manual testing with a colleague here at Salesforce.com in a Master-Master setting, I'll setup a formal review now so we can target the comments better.

        Show
        Lars Hofhansl added a comment - ReadFields is documented at the interface level (whether it is internally versioned should be of no interest to callers). Subclassing VersionedWritable has no advantage here (we can't use super.readFields nor super.write), so I'd rather not leak the versioning stuff out of HLogKey via a getVersion() method until we need it. My main concern is w.r.t. the changes to HRegion, because that now has the cluster UUID sprinkled all over. All tests pass now, and I did some manual testing with a colleague here at Salesforce.com in a Master-Master setting, I'll setup a formal review now so we can target the comments better.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/
        -----------------------------------------------------------

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary
        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and
        (1) do not replicate this data back to the originator and
        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.
        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs


        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing
        -------

        All tests pass.
        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/
        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48.646105)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary
        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and
        (1) do not replicate this data back to the originator and
        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.
        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs


        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing (updated)
        -------

        All tests pass.
        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.
        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48.646105) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing (updated) ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1777
        -----------------------------------------------------------

        Slight optimization outside the scope of this JIRA:
        I think the index for the loop in ReplicationSource.removeNonReplicableEdits()
        for (int i = 0; i < edit.size(); i++) {
        should start from end of kvs and decrement.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4053>

        @Override should be added.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4054>

        @Override should be added.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
        <https://reviews.apache.org/r/1730/#comment4055>

        Should be ' is set'.

        • Ted

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1777 ----------------------------------------------------------- Slight optimization outside the scope of this JIRA: I think the index for the loop in ReplicationSource.removeNonReplicableEdits() for (int i = 0; i < edit.size(); i++) { should start from end of kvs and decrement. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4053 > @Override should be added. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4054 > @Override should be added. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java < https://reviews.apache.org/r/1730/#comment4055 > Should be ' is set'. Ted On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1780
        -----------------------------------------------------------

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java
        <https://reviews.apache.org/r/1730/#comment4085>

        This is good.

        One thought is if these new constants – the key name and the default cluster id – belong in the general HConstants. Are they used across packages? If so, then this is the place for them. Otherwise, I'd say put them in replication and then they'll have the replication class prefix when referred too which will give them some added context. No biggie. I'm just reacting against this fat HConstants.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        <https://reviews.apache.org/r/1730/#comment4086>

        You can't do much better than this if its UUIDs we are carrying.

        Is this code repeated in the Put? If so, might want to factor it out?

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        <https://reviews.apache.org/r/1730/#comment4087>

        Ditto on if repeating code.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java
        <https://reviews.apache.org/r/1730/#comment4088>

        Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java
        <https://reviews.apache.org/r/1730/#comment4089>

        Is this needed?

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        <https://reviews.apache.org/r/1730/#comment4090>

        What is happening here Lars? Are we passing the clusterid from the HRegionServer down to the WAL by writing it into the Delete or Put? Do we have to do that? Can we not pass the clusterid to the WAL directly when HRegionServer creates one?

        On above, nvm, I think I figure out why you do it this way later.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        <https://reviews.apache.org/r/1730/#comment4091>

        Yeah, can't we pass the cluserid in on the HLog constructor rather than per append or do we need to allow for different clusterids coming in via append?

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4092>

        Yeah, if the clusterids are different, i'd think the edits are different? That looks like oversight in original code.

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4093>

        We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past)

        Why not VersionedWritable since it does this version stuff for you?

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4094>

        Why double read of vint? Is this how you do vints?

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java
        <https://reviews.apache.org/r/1730/#comment4095>

        How is this going to be done now? Where do we get cluserid?

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
        <https://reviews.apache.org/r/1730/#comment4096>

        Hmm... oh, I see. The edit could have come in from other than HRegionServer receiving from client on local cluster. OK.

        • Michael

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1780 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java < https://reviews.apache.org/r/1730/#comment4085 > This is good. One thought is if these new constants – the key name and the default cluster id – belong in the general HConstants. Are they used across packages? If so, then this is the place for them. Otherwise, I'd say put them in replication and then they'll have the replication class prefix when referred too which will give them some added context. No biggie. I'm just reacting against this fat HConstants. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java < https://reviews.apache.org/r/1730/#comment4086 > You can't do much better than this if its UUIDs we are carrying. Is this code repeated in the Put? If so, might want to factor it out? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java < https://reviews.apache.org/r/1730/#comment4087 > Ditto on if repeating code. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java < https://reviews.apache.org/r/1730/#comment4088 > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java < https://reviews.apache.org/r/1730/#comment4089 > Is this needed? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java < https://reviews.apache.org/r/1730/#comment4090 > What is happening here Lars? Are we passing the clusterid from the HRegionServer down to the WAL by writing it into the Delete or Put? Do we have to do that? Can we not pass the clusterid to the WAL directly when HRegionServer creates one? On above, nvm, I think I figure out why you do it this way later. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java < https://reviews.apache.org/r/1730/#comment4091 > Yeah, can't we pass the cluserid in on the HLog constructor rather than per append or do we need to allow for different clusterids coming in via append? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4092 > Yeah, if the clusterids are different, i'd think the edits are different? That looks like oversight in original code. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4093 > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past) Why not VersionedWritable since it does this version stuff for you? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4094 > Why double read of vint? Is this how you do vints? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java < https://reviews.apache.org/r/1730/#comment4095 > How is this going to be done now? Where do we get cluserid? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java < https://reviews.apache.org/r/1730/#comment4096 > Hmm... oh, I see. The edit could have come in from other than HRegionServer receiving from client on local cluster. OK. Michael On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1781
        -----------------------------------------------------------

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4097>

        Because the first vint is considered version, we need to read the length for encodedRegionName byte array.

        • Ted

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1781 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4097 > Because the first vint is considered version, we need to read the length for encodedRegionName byte array. Ted On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java, line 666

        > <https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666>

        >

        > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there.

        I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this.
        A common superclass maybe "Mutation" would be nice.

        I can make that change too, depending on how good the FB code is. Maybe in a separate jira?

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1400

        > <https://reviews.apache.org/r/1730/diff/2/?file=38152#file38152line1400>

        >

        > What is happening here Lars? Are we passing the clusterid from the HRegionServer down to the WAL by writing it into the Delete or Put? Do we have to do that? Can we not pass the clusterid to the WAL directly when HRegionServer creates one?

        >

        > On above, nvm, I think I figure out why you do it this way later.

        Yes
        In a ring the Cluster Id needs to be passed on to the next link link.

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java, line 1001

        > <https://reviews.apache.org/r/1730/diff/2/?file=38153#file38153line1001>

        >

        > Yeah, can't we pass the cluserid in on the HLog constructor rather than per append or do we need to allow for different clusterids coming in via append?

        Ok.. I'll add a new constructor, then we won't need to call the setter.

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 232

        > <https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line232>

        >

        > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past)

        >

        > Why not VersionedWritable since it does this version stuff for you?

        This was a good idea from Ted.
        The first entry in HLogKey is the encodedRegionName which is written by Bytes.writeByteArray.
        The first byte (or two bytes) encode the length of the array.

        So we read that first. If the length is < 0 we know it couldn't have been a valid HLogKey, and hence it must be a new one and this is the now the version.

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 235

        > <https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line235>

        >

        > Why double read of vint? Is this how you do vints?

        This one is now the length encodedRegionName

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java, line 374

        > <https://reviews.apache.org/r/1730/diff/2/?file=38148#file38148line374>

        >

        > This is good.

        >

        > One thought is if these new constants – the key name and the default cluster id – belong in the general HConstants. Are they used across packages? If so, then this is the place for them. Otherwise, I'd say put them in replication and then they'll have the replication class prefix when referred too which will give them some added context. No biggie. I'm just reacting against this fat HConstants.

        I think DEFAULT_CLUSTER_ID belongs here.
        CLUSTER_ID_ATTR should go somewhere else I agree. Best would be that common superclass to Get and Put you mention below.

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java, line 84

        > <https://reviews.apache.org/r/1730/diff/2/?file=38151#file38151line84>

        >

        > Is this needed?

        Nope that was used outside of the constructor

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 182

        > <https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line182>

        >

        > Yeah, if the clusterids are different, i'd think the edits are different? That looks like oversight in original code.

        I was not sure here, what does it means for two log keys to be different. I just noticed that he cluster id was missing.
        We seems to use this specifically for sorting, maybe HLogKey even with different cluster id need to order the same?

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java, line 709

        > <https://reviews.apache.org/r/1730/diff/2/?file=38155#file38155line709>

        >

        > How is this going to be done now? Where do we get cluserid?

        What ZooKeeper stores is now no longer considered a cluster id, but just a local numbering scheme for the ZNodes.
        The actual cluster UUID of the sink is read from sink's zookeeper in the main loop in ReplicationSource

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1780
        -----------------------------------------------------------

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java , line 666 > < https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666 > > > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there. I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this. A common superclass maybe "Mutation" would be nice. I can make that change too, depending on how good the FB code is. Maybe in a separate jira? On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java , line 1400 > < https://reviews.apache.org/r/1730/diff/2/?file=38152#file38152line1400 > > > What is happening here Lars? Are we passing the clusterid from the HRegionServer down to the WAL by writing it into the Delete or Put? Do we have to do that? Can we not pass the clusterid to the WAL directly when HRegionServer creates one? > > On above, nvm, I think I figure out why you do it this way later. Yes In a ring the Cluster Id needs to be passed on to the next link link. On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java , line 1001 > < https://reviews.apache.org/r/1730/diff/2/?file=38153#file38153line1001 > > > Yeah, can't we pass the cluserid in on the HLog constructor rather than per append or do we need to allow for different clusterids coming in via append? Ok.. I'll add a new constructor, then we won't need to call the setter. On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 232 > < https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line232 > > > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past) > > Why not VersionedWritable since it does this version stuff for you? This was a good idea from Ted. The first entry in HLogKey is the encodedRegionName which is written by Bytes.writeByteArray. The first byte (or two bytes) encode the length of the array. So we read that first. If the length is < 0 we know it couldn't have been a valid HLogKey, and hence it must be a new one and this is the now the version. On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 235 > < https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line235 > > > Why double read of vint? Is this how you do vints? This one is now the length encodedRegionName On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java , line 374 > < https://reviews.apache.org/r/1730/diff/2/?file=38148#file38148line374 > > > This is good. > > One thought is if these new constants – the key name and the default cluster id – belong in the general HConstants. Are they used across packages? If so, then this is the place for them. Otherwise, I'd say put them in replication and then they'll have the replication class prefix when referred too which will give them some added context. No biggie. I'm just reacting against this fat HConstants. I think DEFAULT_CLUSTER_ID belongs here. CLUSTER_ID_ATTR should go somewhere else I agree. Best would be that common superclass to Get and Put you mention below. On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java , line 84 > < https://reviews.apache.org/r/1730/diff/2/?file=38151#file38151line84 > > > Is this needed? Nope that was used outside of the constructor On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 182 > < https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line182 > > > Yeah, if the clusterids are different, i'd think the edits are different? That looks like oversight in original code. I was not sure here, what does it means for two log keys to be different. I just noticed that he cluster id was missing. We seems to use this specifically for sorting, maybe HLogKey even with different cluster id need to order the same? On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java , line 709 > < https://reviews.apache.org/r/1730/diff/2/?file=38155#file38155line709 > > > How is this going to be done now? Where do we get cluserid? What ZooKeeper stores is now no longer considered a cluster id, but just a local numbering scheme for the ZNodes. The actual cluster UUID of the sink is read from sink's zookeeper in the main loop in ReplicationSource Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1780 ----------------------------------------------------------- On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 232

        > <https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line232>

        >

        > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past)

        >

        > Why not VersionedWritable since it does this version stuff for you?

        Lars Hofhansl wrote:

        This was a good idea from Ted.

        The first entry in HLogKey is the encodedRegionName which is written by Bytes.writeByteArray.

        The first byte (or two bytes) encode the length of the array.

        So we read that first. If the length is < 0 we know it couldn't have been a valid HLogKey, and hence it must be a new one and this is the now the version.

        That sounds like a good idea to me too. Does it need a comment so you and Ted don' t have to answer same question everytime some else looks at this bit of code?

        Good stuff Lars.

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java, line 666

        > <https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666>

        >

        > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there.

        Lars Hofhansl wrote:

        I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this.

        A common superclass maybe "Mutation" would be nice.

        I can make that change too, depending on how good the FB code is. Maybe in a separate jira?

        I notice that Put and Delete implement Operation (The Riley slow-query change has gone in already). Could you put the dup code up in there?

        • Michael

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1780
        -----------------------------------------------------------

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 232 > < https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line232 > > > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past) > > Why not VersionedWritable since it does this version stuff for you? Lars Hofhansl wrote: This was a good idea from Ted. The first entry in HLogKey is the encodedRegionName which is written by Bytes.writeByteArray. The first byte (or two bytes) encode the length of the array. So we read that first. If the length is < 0 we know it couldn't have been a valid HLogKey, and hence it must be a new one and this is the now the version. That sounds like a good idea to me too. Does it need a comment so you and Ted don' t have to answer same question everytime some else looks at this bit of code? Good stuff Lars. On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java , line 666 > < https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666 > > > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there. Lars Hofhansl wrote: I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this. A common superclass maybe "Mutation" would be nice. I can make that change too, depending on how good the FB code is. Maybe in a separate jira? I notice that Put and Delete implement Operation (The Riley slow-query change has gone in already). Could you put the dup code up in there? Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1780 ----------------------------------------------------------- On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 03:59:28, Ted Yu wrote:

        > Slight optimization outside the scope of this JIRA:

        > I think the index for the loop in ReplicationSource.removeNonReplicableEdits()

        > for (int i = 0; i < edit.size(); i++) {

        > should start from end of kvs and decrement.

        Because of the remove from the ArrayList... Good point.
        I'll fine another jira, unless you want to.

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1777
        -----------------------------------------------------------

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 03:59:28, Ted Yu wrote: > Slight optimization outside the scope of this JIRA: > I think the index for the loop in ReplicationSource.removeNonReplicableEdits() > for (int i = 0; i < edit.size(); i++) { > should start from end of kvs and decrement. Because of the remove from the ArrayList... Good point. I'll fine another jira, unless you want to. Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1777 ----------------------------------------------------------- On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java, line 84

        > <https://reviews.apache.org/r/1730/diff/2/?file=38151#file38151line84>

        >

        > Is this needed?

        Lars Hofhansl wrote:

        Nope that was used outside of the constructor

        I meant: never used outside of the constructor

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java, line 666

        > <https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666>

        >

        > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there.

        Lars Hofhansl wrote:

        I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this.

        A common superclass maybe "Mutation" would be nice.

        I can make that change too, depending on how good the FB code is. Maybe in a separate jira?

        Michael Stack wrote:

        I notice that Put and Delete implement Operation (The Riley slow-query change has gone in already). Could you put the dup code up in there?

        He, I was thinking about that too.

        Operation is also shared by Get, Scan, and MultiPut, though.
        Get and Scan do have attributes and MultiPut does not.

        The cluster id part really only makes sense for Put and Delete.

        It almost seems we want two abstraction:
        1. Attributes. These are common to Put, Delete, Get, and Scan. (Now that I look at it, I can't believe how often the same code is repeated for those ). Maybe have a "WithAttributes" class for the lack of a better name?
        2. Cluster id, common to Put and Delete.

        Put and Delete also have other stuff in common (FamilyMap, writeToWal, the row, the rowLock, toMap is very similar, etc, etc.)

        Is there a jira for the FB change?

        So for now... Get this checked in the way it is. Then have additional jira that refactor Put/Delete and maybe Get/Scan/Multiput?

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1780
        -----------------------------------------------------------

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java , line 84 > < https://reviews.apache.org/r/1730/diff/2/?file=38151#file38151line84 > > > Is this needed? Lars Hofhansl wrote: Nope that was used outside of the constructor I meant: never used outside of the constructor On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java , line 666 > < https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666 > > > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there. Lars Hofhansl wrote: I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this. A common superclass maybe "Mutation" would be nice. I can make that change too, depending on how good the FB code is. Maybe in a separate jira? Michael Stack wrote: I notice that Put and Delete implement Operation (The Riley slow-query change has gone in already). Could you put the dup code up in there? He, I was thinking about that too. Operation is also shared by Get, Scan, and MultiPut, though. Get and Scan do have attributes and MultiPut does not. The cluster id part really only makes sense for Put and Delete. It almost seems we want two abstraction: 1. Attributes. These are common to Put, Delete, Get, and Scan. (Now that I look at it, I can't believe how often the same code is repeated for those ). Maybe have a "WithAttributes" class for the lack of a better name? 2. Cluster id, common to Put and Delete. Put and Delete also have other stuff in common (FamilyMap, writeToWal, the row, the rowLock, toMap is very similar, etc, etc.) Is there a jira for the FB change? So for now... Get this checked in the way it is. Then have additional jira that refactor Put/Delete and maybe Get/Scan/Multiput? Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1780 ----------------------------------------------------------- On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 232

        > <https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line232>

        >

        > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past)

        >

        > Why not VersionedWritable since it does this version stuff for you?

        Lars Hofhansl wrote:

        This was a good idea from Ted.

        The first entry in HLogKey is the encodedRegionName which is written by Bytes.writeByteArray.

        The first byte (or two bytes) encode the length of the array.

        So we read that first. If the length is < 0 we know it couldn't have been a valid HLogKey, and hence it must be a new one and this is the now the version.

        Michael Stack wrote:

        That sounds like a good idea to me too. Does it need a comment so you and Ted don' t have to answer same question everytime some else looks at this bit of code?

        Good stuff Lars.

        Yep... I'll add a comment.

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1780
        -----------------------------------------------------------

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 232 > < https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line232 > > > We could have read garbage that happened to be <0? But yeah, this is going to go out with new major version and we'll just pronounce that it will not be able to read old WALs (this we've said in the past) > > Why not VersionedWritable since it does this version stuff for you? Lars Hofhansl wrote: This was a good idea from Ted. The first entry in HLogKey is the encodedRegionName which is written by Bytes.writeByteArray. The first byte (or two bytes) encode the length of the array. So we read that first. If the length is < 0 we know it couldn't have been a valid HLogKey, and hence it must be a new one and this is the now the version. Michael Stack wrote: That sounds like a good idea to me too. Does it need a comment so you and Ted don' t have to answer same question everytime some else looks at this bit of code? Good stuff Lars. Yep... I'll add a comment. Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1780 ----------------------------------------------------------- On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 182

        > <https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line182>

        >

        > Yeah, if the clusterids are different, i'd think the edits are different? That looks like oversight in original code.

        Lars Hofhansl wrote:

        I was not sure here, what does it means for two log keys to be different. I just noticed that he cluster id was missing.

        We seems to use this specifically for sorting, maybe HLogKey even with different cluster id need to order the same?

        Should HLogKey should sort by sequenceid?

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java, line 1001

        > <https://reviews.apache.org/r/1730/diff/2/?file=38153#file38153line1001>

        >

        > Yeah, can't we pass the cluserid in on the HLog constructor rather than per append or do we need to allow for different clusterids coming in via append?

        Lars Hofhansl wrote:

        Ok.. I'll add a new constructor, then we won't need to call the setter.

        Well, sounds like you don't need the constructor. I was only suggesting it if clusterid was same for all edits that come into the WAL but later I saw that the edits can come from another cluster. If you added clusterid to constructor, what would you do? Add the passed clusterid to the WAL IFF the passed Delete or Put were absent a clusterid?

        On 2011-09-07 06:01:09, Michael Stack wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java, line 666

        > <https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666>

        >

        > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there.

        Lars Hofhansl wrote:

        I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this.

        A common superclass maybe "Mutation" would be nice.

        I can make that change too, depending on how good the FB code is. Maybe in a separate jira?

        Michael Stack wrote:

        I notice that Put and Delete implement Operation (The Riley slow-query change has gone in already). Could you put the dup code up in there?

        Lars Hofhansl wrote:

        He, I was thinking about that too.

        Operation is also shared by Get, Scan, and MultiPut, though.

        Get and Scan do have attributes and MultiPut does not.

        The cluster id part really only makes sense for Put and Delete.

        It almost seems we want two abstraction:

        1. Attributes. These are common to Put, Delete, Get, and Scan. (Now that I look at it, I can't believe how often the same code is repeated for those ). Maybe have a "WithAttributes" class for the lack of a better name?

        2. Cluster id, common to Put and Delete.

        Put and Delete also have other stuff in common (FamilyMap, writeToWal, the row, the rowLock, toMap is very similar, etc, etc.)

        Is there a jira for the FB change?

        So for now... Get this checked in the way it is. Then have additional jira that refactor Put/Delete and maybe Get/Scan/Multiput?

        On 1., I'd say Attributes rather than WithAttributes as the added class name (I think adding it would be good but can be in a different issue I'd say).

        On 2., you talking about a ClusterId or should it be a Replication Interface? If an Interface, you can mix it in easily. But then you don't have a super class to put the commonality into. Back to square one (smile). Or some utility that took the Interface and it did the set and get of clusterid.

        But if Put and Delete have other commonality, that'd be cool figuring a superclass they could share (Result too?)

        But now we are into a different JIRA? Don't you think? Yes, lets do the refactor elsewhere. The duplicated code don't look so bad after the above exercise teasing stuff out.

        The JIRA that added Operation is:

        HBASE-4117 Slow Query Log and Client Operation Fingerprints
        (Riley Patterson)

        • Michael

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1780
        -----------------------------------------------------------

        On 2011-09-06 23:16:48, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-06 23:16:48)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 182 > < https://reviews.apache.org/r/1730/diff/2/?file=38154#file38154line182 > > > Yeah, if the clusterids are different, i'd think the edits are different? That looks like oversight in original code. Lars Hofhansl wrote: I was not sure here, what does it means for two log keys to be different. I just noticed that he cluster id was missing. We seems to use this specifically for sorting, maybe HLogKey even with different cluster id need to order the same? Should HLogKey should sort by sequenceid? On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java , line 1001 > < https://reviews.apache.org/r/1730/diff/2/?file=38153#file38153line1001 > > > Yeah, can't we pass the cluserid in on the HLog constructor rather than per append or do we need to allow for different clusterids coming in via append? Lars Hofhansl wrote: Ok.. I'll add a new constructor, then we won't need to call the setter. Well, sounds like you don't need the constructor. I was only suggesting it if clusterid was same for all edits that come into the WAL but later I saw that the edits can come from another cluster. If you added clusterid to constructor, what would you do? Add the passed clusterid to the WAL IFF the passed Delete or Put were absent a clusterid? On 2011-09-07 06:01:09, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java , line 666 > < https://reviews.apache.org/r/1730/diff/2/?file=38150#file38150line666 > > > Hmm.. there is an fb patch that gives Get/Delete, etc. a common heritage. Let me dig it up and commit so you can put this common code there. Lars Hofhansl wrote: I was thinking about this. Then I thought the code duplication is not significant enough. But now I realize Put and Delete already have a lot in common that could be factored out in addition to this. A common superclass maybe "Mutation" would be nice. I can make that change too, depending on how good the FB code is. Maybe in a separate jira? Michael Stack wrote: I notice that Put and Delete implement Operation (The Riley slow-query change has gone in already). Could you put the dup code up in there? Lars Hofhansl wrote: He, I was thinking about that too. Operation is also shared by Get, Scan, and MultiPut, though. Get and Scan do have attributes and MultiPut does not. The cluster id part really only makes sense for Put and Delete. It almost seems we want two abstraction: 1. Attributes. These are common to Put, Delete, Get, and Scan. (Now that I look at it, I can't believe how often the same code is repeated for those ). Maybe have a "WithAttributes" class for the lack of a better name? 2. Cluster id, common to Put and Delete. Put and Delete also have other stuff in common (FamilyMap, writeToWal, the row, the rowLock, toMap is very similar, etc, etc.) Is there a jira for the FB change? So for now... Get this checked in the way it is. Then have additional jira that refactor Put/Delete and maybe Get/Scan/Multiput? On 1., I'd say Attributes rather than WithAttributes as the added class name (I think adding it would be good but can be in a different issue I'd say). On 2., you talking about a ClusterId or should it be a Replication Interface? If an Interface, you can mix it in easily. But then you don't have a super class to put the commonality into. Back to square one (smile). Or some utility that took the Interface and it did the set and get of clusterid. But if Put and Delete have other commonality, that'd be cool figuring a superclass they could share (Result too?) But now we are into a different JIRA? Don't you think? Yes, lets do the refactor elsewhere. The duplicated code don't look so bad after the above exercise teasing stuff out. The JIRA that added Operation is: HBASE-4117 Slow Query Log and Client Operation Fingerprints (Riley Patterson) Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1780 ----------------------------------------------------------- On 2011-09-06 23:16:48, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-06 23:16:48) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1165864 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/
        -----------------------------------------------------------

        (Updated 2011-09-07 18:49:31.631269)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Changes
        -------

        Added comments.
        Made originating-cluster-only operations (checkAndMutate, ICV) explicit.

        Summary
        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and
        (1) do not replicate this data back to the originator and
        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.
        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs (updated)


        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing
        -------

        All tests pass.
        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.
        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 18:49:31.631269) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Changes ------- Added comments. Made originating-cluster-only operations (checkAndMutate, ICV) explicit. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1797
        -----------------------------------------------------------

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
        <https://reviews.apache.org/r/1730/#comment4140>

        fix sentence

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
        <https://reviews.apache.org/r/1730/#comment4141>

        maybe have a more helpful message

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
        <https://reviews.apache.org/r/1730/#comment4142>

        seems to me there's an opportunity for refactoring with TestReplication

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
        <https://reviews.apache.org/r/1730/#comment4143>

        fix those brackets in both methods

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
        <https://reviews.apache.org/r/1730/#comment4144>

        that's clever

        • Jean-Daniel

        On 2011-09-07 18:49:31, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-07 18:49:31)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1797 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java < https://reviews.apache.org/r/1730/#comment4140 > fix sentence http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java < https://reviews.apache.org/r/1730/#comment4141 > maybe have a more helpful message http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java < https://reviews.apache.org/r/1730/#comment4142 > seems to me there's an opportunity for refactoring with TestReplication http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java < https://reviews.apache.org/r/1730/#comment4143 > fix those brackets in both methods http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java < https://reviews.apache.org/r/1730/#comment4144 > that's clever Jean-Daniel On 2011-09-07 18:49:31, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 18:49:31) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 18:51:35, Jean-Daniel Cryans wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java, line 96

        > <https://reviews.apache.org/r/1730/diff/2/?file=38159#file38159line96>

        >

        > seems to me there's an opportunity for refactoring with TestReplication

        Yes, although I think the two will move in different directions over time.
        For example I want to add testing with three clusters to setup a cycle soon, and then this would all change.

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1797
        -----------------------------------------------------------

        On 2011-09-07 18:49:31, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-07 18:49:31)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 18:51:35, Jean-Daniel Cryans wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java , line 96 > < https://reviews.apache.org/r/1730/diff/2/?file=38159#file38159line96 > > > seems to me there's an opportunity for refactoring with TestReplication Yes, although I think the two will move in different directions over time. For example I want to add testing with three clusters to setup a cycle soon, and then this would all change. Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1797 ----------------------------------------------------------- On 2011-09-07 18:49:31, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 18:49:31) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/
        -----------------------------------------------------------

        (Updated 2011-09-07 19:19:06.781064)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Changes
        -------

        Addressed J-D issues

        Summary
        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and
        (1) do not replicate this data back to the originator and
        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.
        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs (updated)


        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing
        -------

        All tests pass.
        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.
        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 19:19:06.781064) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Changes ------- Addressed J-D issues Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        Ted Yu added a comment -

        The change to removeNonReplicableEdits() is included in version 10.

        Show
        Ted Yu added a comment - The change to removeNonReplicableEdits() is included in version 10.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1801
        -----------------------------------------------------------

        Ship it!

        • Michael

        On 2011-09-07 19:19:06, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-07 19:19:06)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1801 ----------------------------------------------------------- Ship it! Michael On 2011-09-07 19:19:06, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 19:19:06) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        stack added a comment -

        Lars you good on Ted's mod? Its this:

        15:44 < tyu> a small change for removeNonReplicableEdits()
        15:45 < tyu>    protected void removeNonReplicableEdits(WALEdit edit) {
        15:45 < tyu>      NavigableMap<byte[], Integer> scopes = edit.getScopes();
        15:45 < tyu>      List<KeyValue> kvs = edit.getKeyValues();
        15:45 < tyu> -    for (int i = 0; i < edit.size(); i++) {
        15:45 < tyu> +    for (int i = edit.size()-1; i >= 0; i--) {
        15:45 < tyu>          kvs.remove(i);
        15:45 < tyu> -        i--;
        
        Show
        stack added a comment - Lars you good on Ted's mod? Its this: 15:44 < tyu> a small change for removeNonReplicableEdits() 15:45 < tyu> protected void removeNonReplicableEdits(WALEdit edit) { 15:45 < tyu> NavigableMap< byte [], Integer > scopes = edit.getScopes(); 15:45 < tyu> List<KeyValue> kvs = edit.getKeyValues(); 15:45 < tyu> - for ( int i = 0; i < edit.size(); i++) { 15:45 < tyu> + for ( int i = edit.size()-1; i >= 0; i--) { 15:45 < tyu> kvs.remove(i); 15:45 < tyu> - i--;
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-07 22:37:48, Michael Stack wrote:

        >

        Cool... Let me add Ted's change, and a test does cyclic replication.

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1801
        -----------------------------------------------------------

        On 2011-09-07 19:19:06, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-07 19:19:06)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-07 22:37:48, Michael Stack wrote: > Cool... Let me add Ted's change, and a test does cyclic replication. Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1801 ----------------------------------------------------------- On 2011-09-07 19:19:06, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 19:19:06) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/
        -----------------------------------------------------------

        (Updated 2011-09-07 23:19:53.225308)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Changes
        -------

        Hopefully the last iteration.
        Integrated Ted's optimization for ReplicationSource.removeNonReplicableEdits.
        Better tests. Now tests Master - Master and a Cycle between 3 clusters.

        Summary
        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and
        (1) do not replicate this data back to the originator and
        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.
        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs (updated)


        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212
        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing
        -------

        All tests pass.
        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.
        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 23:19:53.225308) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Changes ------- Hopefully the last iteration. Integrated Ted's optimization for ReplicationSource.removeNonReplicableEdits. Better tests. Now tests Master - Master and a Cycle between 3 clusters. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1807
        -----------------------------------------------------------

        The new TestMasterReplication passes

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        <https://reviews.apache.org/r/1730/#comment4160>

        I still don't see VersionedWritable.

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
        <https://reviews.apache.org/r/1730/#comment4159>

        A short javadoc should be added, e.g.:

        This class tests Master - Master replication and cyclic replication between 3 clusters.

        Can be done at time of commit.

        • Ted

        On 2011-09-07 23:19:53, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-07 23:19:53)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1807 ----------------------------------------------------------- The new TestMasterReplication passes http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java < https://reviews.apache.org/r/1730/#comment4160 > I still don't see VersionedWritable. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java < https://reviews.apache.org/r/1730/#comment4159 > A short javadoc should be added, e.g.: This class tests Master - Master replication and cyclic replication between 3 clusters. Can be done at time of commit. Ted On 2011-09-07 23:19:53, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 23:19:53) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        Ted Yu added a comment -

        Version 12 makes HLogKey extend VersionedWritable

        Show
        Ted Yu added a comment - Version 12 makes HLogKey extend VersionedWritable
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-08 02:45:44, Ted Yu wrote:

        > The new TestMasterReplication passes

        Of course it does

        On 2011-09-08 02:45:44, Ted Yu wrote:

        > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java, line 45

        > <https://reviews.apache.org/r/1730/diff/5/?file=38669#file38669line45>

        >

        > I still don't see VersionedWritable.

        It does not really make sense to use VersionedWritable here. We can neither use readFields(...) not write(...) of VersionedWritable.

        • Lars

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1730/#review1807
        -----------------------------------------------------------

        On 2011-09-07 23:19:53, Lars Hofhansl wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1730/

        -----------------------------------------------------------

        (Updated 2011-09-07 23:19:53)

        Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans.

        Summary

        -------

        For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and

        (1) do not replicate this data back to the originator and

        (2) pass that information on to the next link in a cycle > 2

        In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink.

        This addresses bug HBASE-2195.

        https://issues.apache.org/jira/browse/HBASE-2195

        Diffs

        -----

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212

        http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212

        Diff: https://reviews.apache.org/r/1730/diff

        Testing

        -------

        All tests pass.

        New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes.

        Manually tested a cycle between 3 clusters.

        Thanks,

        Lars

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-08 02:45:44, Ted Yu wrote: > The new TestMasterReplication passes Of course it does On 2011-09-08 02:45:44, Ted Yu wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java , line 45 > < https://reviews.apache.org/r/1730/diff/5/?file=38669#file38669line45 > > > I still don't see VersionedWritable. It does not really make sense to use VersionedWritable here. We can neither use readFields(...) not write(...) of VersionedWritable. Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/#review1807 ----------------------------------------------------------- On 2011-09-07 23:19:53, Lars Hofhansl wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1730/ ----------------------------------------------------------- (Updated 2011-09-07 23:19:53) Review request for hbase, Ted Yu, Michael Stack, and Jean-Daniel Cryans. Summary ------- For Master <-> Master replication (as well as cycles > 2) a replication sink needs to keep track who is was the originator of a change and (1) do not replicate this data back to the originator and (2) pass that information on to the next link in a cycle > 2 In order to do that, HlogKeys read from the WAL are tagged with the Cluster UUID at the ReplicationSource before the logs are shipped to the Sink. The sink writes the Cluster UUID into the WAL log (in HlogKey, so only once per edit). If the sink is also source for yet another sink the ClusterUUID is retained in the HLogKeys read from the local WAL and passed on to the sink. This addresses bug HBASE-2195 . https://issues.apache.org/jira/browse/HBASE-2195 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java 1166212 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 1166212 Diff: https://reviews.apache.org/r/1730/diff Testing ------- All tests pass. New test: TestMasterReplication (which tests Master <-> Master, but no cycles > 2, yet), also passes. Manually tested a cycle between 3 clusters. Thanks, Lars
        Hide
        Lars Hofhansl added a comment -

        Extending VersionedWritable makes no sense to me. We are not using any of its methods.
        And VersionedWritable is not an interface so now you are enforcing a strict class hierarchy where that is not necessary.

        Show
        Lars Hofhansl added a comment - Extending VersionedWritable makes no sense to me. We are not using any of its methods. And VersionedWritable is not an interface so now you are enforcing a strict class hierarchy where that is not necessary.
        Hide
        Ted Yu added a comment - - edited

        At this moment, we're not using getVersion() of VersionedWritable. But there might be future need for this method.
        Here is an example from ThriftServer.getTableRegions() that uses HRI.getVersion():

                  region.name = ByteBuffer.wrap(regionInfo.getRegionName());
                  region.version = regionInfo.getVersion();
                  regions.add(region);
        

        This JIRA is targeting multi-cluster replication where we should expect different releases of HBase to be in use. Providing HLogKey.getVersion() has its benefits.

        My two cents.

        Show
        Ted Yu added a comment - - edited At this moment, we're not using getVersion() of VersionedWritable. But there might be future need for this method. Here is an example from ThriftServer.getTableRegions() that uses HRI.getVersion(): region.name = ByteBuffer.wrap(regionInfo.getRegionName()); region.version = regionInfo.getVersion(); regions.add(region); This JIRA is targeting multi-cluster replication where we should expect different releases of HBase to be in use. Providing HLogKey.getVersion() has its benefits. My two cents.
        Hide
        stack added a comment -

        You think we should expose this version Ted? I'd think it an internal implementation detail. Hopefully, the case should be that the clients never have to worry what version their HLogKey is. It always just works whatever data format is thrown at it because internally it'll do the right thing instantiating an HLogKey though given version 0 or version 1 data. It seems like HLogKey does what VersionedWritable adds (but for the getVersion method)

        Show
        stack added a comment - You think we should expose this version Ted? I'd think it an internal implementation detail. Hopefully, the case should be that the clients never have to worry what version their HLogKey is. It always just works whatever data format is thrown at it because internally it'll do the right thing instantiating an HLogKey though given version 0 or version 1 data. It seems like HLogKey does what VersionedWritable adds (but for the getVersion method)
        Hide
        Ted Yu added a comment -

        We were lucky to be able to squeeze version into HLogKey this time (version 0 vs. version -1). My expectation is that we may add more fields beyond UUID cluster Id.
        Exposing getVersion() allows ReplicationSink/ReplicationSource to handle WALEdit's coming from clusters running different releases of HBase.
        E.g. FB may run 0.89 in some clusters and 0.90.x in other clusters.

        Michael brought VersionedWritable up @ 04/Sep/11 22:52
        I think we should make use of it.

        Show
        Ted Yu added a comment - We were lucky to be able to squeeze version into HLogKey this time (version 0 vs. version -1). My expectation is that we may add more fields beyond UUID cluster Id. Exposing getVersion() allows ReplicationSink/ReplicationSource to handle WALEdit's coming from clusters running different releases of HBase. E.g. FB may run 0.89 in some clusters and 0.90.x in other clusters. Michael brought VersionedWritable up @ 04/Sep/11 22:52 I think we should make use of it.
        Hide
        Lars Hofhansl added a comment -

        I still think versioning on HLogKey should be kept internal.
        Now that we store a version internally we can add more fields.

        Hopefully Replication

        {Source|Sink}

        never have to know about that.
        If they do, we can add a getVersion() method then.

        See for example WALEdit, which does a similar hack for the version also does not extend VersionedWritable and version is strictly internal.

        What this experience shows is that everything new that is serialized should be versioned from the beginning.

        That all said, I am not emotionally attached to not extending VersionWritable
        Let's just say I am +-0 on this.

        Show
        Lars Hofhansl added a comment - I still think versioning on HLogKey should be kept internal. Now that we store a version internally we can add more fields. Hopefully Replication {Source|Sink} never have to know about that. If they do, we can add a getVersion() method then. See for example WALEdit, which does a similar hack for the version also does not extend VersionedWritable and version is strictly internal. What this experience shows is that everything new that is serialized should be versioned from the beginning. That all said, I am not emotionally attached to not extending VersionWritable Let's just say I am +-0 on this.
        Hide
        stack added a comment -

        @Ted I see what you are saying. But if we add in VersionedWritable, he'll not be able to do his trick where he can deserialize this version of HLogKey and the previous one (since it will read a byte off the stream and treat it as the version regardless). As I see it, we can:

        + Leave patch as is. Then later, if we need to change HLogKey, we'll up the version to '0' ('0' is an unlikely length on an vint for encoded name). So we've squeezed an extra version in. Thereafter we'd have to move to use versions 1, 2, 3, etc. If we ever encountered a WAL that had HLogKeys written before this patch, we'd not be able to deserialize them since we'd confuse them with these newer 1, 2, 3.. versions. I think that should be fine since a good bit of time will have passed between the two version and deserializing old stuff will be less likely.
        + We move to VersionedWritable now but that'll break our being able to read WALs written before this patch went in.

        Like I say, I think the second option above tolerable given as we're going to a major version and we can state we can't read WALs across versions but it seems like leaving the patch as is, option 1, gives us most flexibility (we can read old WALs).

        What you think Ted?

        Show
        stack added a comment - @Ted I see what you are saying. But if we add in VersionedWritable, he'll not be able to do his trick where he can deserialize this version of HLogKey and the previous one (since it will read a byte off the stream and treat it as the version regardless). As I see it, we can: + Leave patch as is. Then later, if we need to change HLogKey, we'll up the version to '0' ('0' is an unlikely length on an vint for encoded name). So we've squeezed an extra version in. Thereafter we'd have to move to use versions 1, 2, 3, etc. If we ever encountered a WAL that had HLogKeys written before this patch, we'd not be able to deserialize them since we'd confuse them with these newer 1, 2, 3.. versions. I think that should be fine since a good bit of time will have passed between the two version and deserializing old stuff will be less likely. + We move to VersionedWritable now but that'll break our being able to read WALs written before this patch went in. Like I say, I think the second option above tolerable given as we're going to a major version and we can state we can't read WALs across versions but it seems like leaving the patch as is, option 1, gives us most flexibility (we can read old WALs). What you think Ted?
        Hide
        Lars Hofhansl added a comment -

        Oh and re: HRegionInfo.
        It is not true that none of the VersionWritable methods are used.
        HRegionInfo.readFields(...) calls super.readFields(...) first and write(...) calls super.write().
        (That is what I meant with "we are not using any of the VersionedWritable methods").

        Show
        Lars Hofhansl added a comment - Oh and re: HRegionInfo. It is not true that none of the VersionWritable methods are used. HRegionInfo.readFields(...) calls super.readFields(...) first and write(...) calls super.write(). (That is what I meant with "we are not using any of the VersionedWritable methods").
        Hide
        Lars Hofhansl added a comment -

        @Stack The trick with with deserializing previous versions would still work; but only because we are not actually using any methods from VersionedWritable (HLogKey.readFields() does not call super.readFields and HLogKey.write does not call super.write.)

        Show
        Lars Hofhansl added a comment - @Stack The trick with with deserializing previous versions would still work; but only because we are not actually using any methods from VersionedWritable (HLogKey.readFields() does not call super.readFields and HLogKey.write does not call super.write.)
        Hide
        stack added a comment -

        The trick with with deserializing previous versions would still work; but only because we are not actually using any methods from VersionedWritable (HLogKey.readFields() does not call super.readFields and HLogKey.write does not call super.write.)

        Yes. Exactly. Given this, I'd say subclassing VersionedWritable would only confuse since you are not making full use of it.

        Show
        stack added a comment - The trick with with deserializing previous versions would still work; but only because we are not actually using any methods from VersionedWritable (HLogKey.readFields() does not call super.readFields and HLogKey.write does not call super.write.) Yes. Exactly. Given this, I'd say subclassing VersionedWritable would only confuse since you are not making full use of it.
        Hide
        Ted Yu added a comment -

        Hopefully Replication{Source|Sink} never have to know about that.

        So far our test implicitly assumes that all the clusters participating in replication are not older than version -1 (i.e. version 0).

        See ReplicationSource.readAllEntriesToReplicateOrNextFile():

        +          if (HConstants.DEFAULT_CLUSTER_ID == logKey.getClusterId()) {
        +            logKey.setClusterId(this.clusterId);
        +          }
        

        I think there might be a problem if version of logKey is 0 in the above case because logKey might not be local.

        Having getVersion() allows us to make better judgment.

        I will publish version 13 of the patch.

        BTW I noticed there is an int version inside readFields() while VERSION is declared final in HLogKey - this happens when two people work on different parts of the same file

        Show
        Ted Yu added a comment - Hopefully Replication{Source|Sink} never have to know about that. So far our test implicitly assumes that all the clusters participating in replication are not older than version -1 (i.e. version 0). See ReplicationSource.readAllEntriesToReplicateOrNextFile(): + if (HConstants.DEFAULT_CLUSTER_ID == logKey.getClusterId()) { + logKey.setClusterId( this .clusterId); + } I think there might be a problem if version of logKey is 0 in the above case because logKey might not be local. Having getVersion() allows us to make better judgment. I will publish version 13 of the patch. BTW I noticed there is an int version inside readFields() while VERSION is declared final in HLogKey - this happens when two people work on different parts of the same file
        Hide
        Lars Hofhansl added a comment -

        Not true Ted. There is no implicit version assumption.
        The assumption is that we can get a cluster UUID from HLogKey, and if no UUID was set we get a reference to DEFAULT_CLUSTER_ID, which is true for any version of the WAL file format, precisely because we kept all the versioning inside of HLogKey.

        The new code can read the old file. Of course you cannot mix the new ReplicationSource code with the old HLogKey code, but that is not the goal of versioning here.

        Show
        Lars Hofhansl added a comment - Not true Ted. There is no implicit version assumption. The assumption is that we can get a cluster UUID from HLogKey, and if no UUID was set we get a reference to DEFAULT_CLUSTER_ID, which is true for any version of the WAL file format, precisely because we kept all the versioning inside of HLogKey. The new code can read the old file. Of course you cannot mix the new ReplicationSource code with the old HLogKey code, but that is not the goal of versioning here.
        Hide
        Ted Yu added a comment -

        Version 13 of the patch.
        HLogKey.readFields() sets version field.
        I also added check in ReplicationSource.readAllEntriesToReplicateOrNextFile()

        Show
        Ted Yu added a comment - Version 13 of the patch. HLogKey.readFields() sets version field. I also added check in ReplicationSource.readAllEntriesToReplicateOrNextFile()
        Hide
        Lars Hofhansl added a comment -

        HLogKey.version is still static, v13 won't work.
        This is supposed to be the static version of the class, not the instance.

        I maintain that the version check in readAllEntriesToReplicateOrNextFile() is neither needed nor (IMHO) desired.

        On the good side if we quibbling over this detail the rest of the patch seems to be OK

        Show
        Lars Hofhansl added a comment - HLogKey.version is still static, v13 won't work. This is supposed to be the static version of the class , not the instance. I maintain that the version check in readAllEntriesToReplicateOrNextFile() is neither needed nor (IMHO) desired. On the good side if we quibbling over this detail the rest of the patch seems to be OK
        Hide
        Ted Yu added a comment -

        Version 14 of the patch makes HLogKey.version non-static. The reason is that version from old log files should be different from -1.
        The version of WALEdit is not static, either.

        I removed the version check in readAllEntriesToReplicateOrNextFile() which merely served as demonstration.

        Show
        Ted Yu added a comment - Version 14 of the patch makes HLogKey.version non-static. The reason is that version from old log files should be different from -1. The version of WALEdit is not static, either. I removed the version check in readAllEntriesToReplicateOrNextFile() which merely served as demonstration.
        Hide
        Lars Hofhansl added a comment -

        The version in WALEdit is final, though, and there is no constructor setting it.
        The fact that it is not static looks like a oversight.

        I disagree with the latest patch. The version in HLogKey should be a class version. That is the whole point of VersionedWritable.

        Having an instance version only makes sense if the write would behave differently based on version.

        I prefer patch v11, where HLogKey does not extend VersionedWritable. Just my $0.02.

        Show
        Lars Hofhansl added a comment - The version in WALEdit is final, though, and there is no constructor setting it. The fact that it is not static looks like a oversight. I disagree with the latest patch. The version in HLogKey should be a class version. That is the whole point of VersionedWritable. Having an instance version only makes sense if the write would behave differently based on version. I prefer patch v11, where HLogKey does not extend VersionedWritable. Just my $0.02.
        Hide
        Ted Yu added a comment -

        The special handling in readFields() already makes HLogKey deviate from a single class version. Meaning when this.clusterId carries HConstants.DEFAULT_CLUSTER_ID, there is a chance that the HLogKey wasn't generated in local cluster.
        I think that's why we call it DEFAULT_CLUSTER_ID instead of LOCAL_CLUSTER_ID.

        We can create our own VersionedWritable interface (maybe with a different name) which HLogKey can implement.

        Show
        Ted Yu added a comment - The special handling in readFields() already makes HLogKey deviate from a single class version. Meaning when this.clusterId carries HConstants.DEFAULT_CLUSTER_ID, there is a chance that the HLogKey wasn't generated in local cluster. I think that's why we call it DEFAULT_CLUSTER_ID instead of LOCAL_CLUSTER_ID. We can create our own VersionedWritable interface (maybe with a different name) which HLogKey can implement.
        Hide
        Lars Hofhansl added a comment -

        The entries in a log file can have different versions. The class is either (implicit) version 0 in 0.90.x or version -1 0.92. It should not be a member variable.

        We have the class version so that we can decide how to tag the write side of thing and then decide at read time how to behave. Every versioned class has a static version (or a final that is never set by a constructor which amount to the same behavior).

        Let's please settle on v11 of the patch.
        We can have a general discussion about versioning outside of this patch.

        (If anything it shows that everything that is written to a file needs to be versioned from the beginning).

        Show
        Lars Hofhansl added a comment - The entries in a log file can have different versions. The class is either (implicit) version 0 in 0.90.x or version -1 0.92. It should not be a member variable. We have the class version so that we can decide how to tag the write side of thing and then decide at read time how to behave. Every versioned class has a static version (or a final that is never set by a constructor which amount to the same behavior). Let's please settle on v11 of the patch. We can have a general discussion about versioning outside of this patch. (If anything it shows that everything that is written to a file needs to be versioned from the beginning).
        Hide
        stack added a comment -

        Let's please settle on v11 of the patch. We can have a general discussion about versioning outside of this patch.

        I'm +1 on doing this

        Show
        stack added a comment - Let's please settle on v11 of the patch. We can have a general discussion about versioning outside of this patch. I'm +1 on doing this
        Hide
        Lars Hofhansl added a comment -

        Based on a suggestion by Ted I tested the following scenario (with patch v11): Start with hbase-0.90.x, upgrade and restart with trunk (with 4301 applied), and set up master-master replication with another trunk cluster. Make sure replication works fine (and it does)

        In terms of versioning...
        When we did versioning for a project I used to work on a long time ago, we had maintained a class version and an instance version. We might need both.
        The class should know which version it is, but also it is useful in many scenarios to ask the instance what specific version it is on.
        I.e. we need an interface with two methods: One to get the class version, one to get the instance version.
        I think this is for a different patch, though.

        Show
        Lars Hofhansl added a comment - Based on a suggestion by Ted I tested the following scenario (with patch v11): Start with hbase-0.90.x, upgrade and restart with trunk (with 4301 applied), and set up master-master replication with another trunk cluster. Make sure replication works fine (and it does) In terms of versioning... When we did versioning for a project I used to work on a long time ago, we had maintained a class version and an instance version. We might need both. The class should know which version it is, but also it is useful in many scenarios to ask the instance what specific version it is on. I.e. we need an interface with two methods: One to get the class version, one to get the instance version. I think this is for a different patch, though.
        Hide
        Ted Yu added a comment -

        I created HBASE-4353 for the new VersionedWritable interface.
        The remaining work can be done there.

        Thanks for the perseverance Lars.

        Show
        Ted Yu added a comment - I created HBASE-4353 for the new VersionedWritable interface. The remaining work can be done there. Thanks for the perseverance Lars.
        Hide
        stack added a comment -

        Are we good to go then?

        Show
        stack added a comment - Are we good to go then?
        Hide
        Ted Yu added a comment -

        Yes.

        Show
        Ted Yu added a comment - Yes.
        Hide
        Jean-Daniel Cryans added a comment -

        +1

        Show
        Jean-Daniel Cryans added a comment - +1
        Hide
        stack added a comment -

        Where is v11? Its whats up on rb?

        Show
        stack added a comment - Where is v11? Its whats up on rb?
        Hide
        Ted Yu added a comment -

        Yes.

        Show
        Ted Yu added a comment - Yes.
        Hide
        Lars Hofhansl added a comment -

        Yep, the one of rb.

        Show
        Lars Hofhansl added a comment - Yep, the one of rb.
        Hide
        Ted Yu added a comment -

        Integrated v11 to TRUNK.

        Thanks for the patch Lars.
        Thanks for the review Michael and J-D.

        Show
        Ted Yu added a comment - Integrated v11 to TRUNK. Thanks for the patch Lars. Thanks for the review Michael and J-D.
        Hide
        Lars Hofhansl added a comment -

        Thanks for the thorough review Ted. And thanks to Stack and J-D for your review.

        Show
        Lars Hofhansl added a comment - Thanks for the thorough review Ted. And thanks to Stack and J-D for your review.
        Hide
        Lars Hofhansl added a comment -

        Where should this be documented?

        Show
        Lars Hofhansl added a comment - Where should this be documented?
        Show
        Ted Yu added a comment - http://hbase.apache.org/replication.html
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2191 (See https://builds.apache.org/job/HBase-TRUNK/2191/)
        HBASE-2195 Support cyclic replication (Lars Hofhansl)

        tedyu :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2191 (See https://builds.apache.org/job/HBase-TRUNK/2191/ ) HBASE-2195 Support cyclic replication (Lars Hofhansl) tedyu : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeper.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogMethods.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
        Hide
        Lars Hofhansl added a comment -

        Should CopyTable be made aware of Master <-> Master scenarios as well?

        Otherwise everything that CopyTable copies from MasterI to MasterII is replicated back to the MasterI once.
        At least maybe it should be added to the documentation (i.e. setup Master <-> Master replication after CopyTable is finished).

        Show
        Lars Hofhansl added a comment - Should CopyTable be made aware of Master <-> Master scenarios as well? Otherwise everything that CopyTable copies from MasterI to MasterII is replicated back to the MasterI once. At least maybe it should be added to the documentation (i.e. setup Master <-> Master replication after CopyTable is finished).
        Hide
        Jean-Daniel Cryans added a comment -

        I guess we could add some wits... maybe even verify beforehand the definition of each table and see if there's a problem.

        Show
        Jean-Daniel Cryans added a comment - I guess we could add some wits... maybe even verify beforehand the definition of each table and see if there's a problem.

          People

          • Assignee:
            Lars Hofhansl
            Reporter:
            Jean-Daniel Cryans
          • Votes:
            4 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development