[jira] [Commented] (HBASE-21312) ReplicationSource should log the incorrect peerClusterId at Error level instead of Info

["Wellington Chevreuil (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HBASE-21312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16859340#comment-16859340 ] 

Wellington Chevreuil commented on HBASE-21312:
----------------------------------------------

Hi [~lingesh],

Thanks for the contribution! Some observations on this v2 patch:

1) It seems that the added test is not enforcing the new behaviour, had run it against the original version of ReplicationSource that doesn't pass ReplicationException to _terminate_ method, and the test still passed. I think the problem is because it's only asserting the condition when the message is in the log (line #394), but when we don't log the message, the test never enter the _if_ statement from line #392, and never calls any further _assert_, which then makes it complete successful.

2) Noticed you had fixed some pre-existing checkstyles issues. For better tracking purposes, we should ideally address those on specific jiras for checkstyles issues, leaving this patch only with changes relevant for this Jira purpose.

3) Noticed this latest patch has been created from a _diff_ command. While this is ok for reporting all the changes properly, _git_ provides _format-patch_ command, that will insert additional info about the author and commit message, along with the diff itself. Useful info on how to format/create patches is available [here|[https://hbase.apache.org/book.html#submitting.patches.create]]. 

> ReplicationSource should log the incorrect peerClusterId at Error level instead of Info
> ---------------------------------------------------------------------------------------
>
>                 Key: HBASE-21312
>                 URL: https://issues.apache.org/jira/browse/HBASE-21312
>             Project: HBase
>          Issue Type: Improvement
>          Components: Replication
>    Affects Versions: 2.0.2, 1.4.7
>            Reporter: Lingeshwaran Radhakrishnan
>            Assignee: Lingeshwaran Radhakrishnan
>            Priority: Minor
>              Labels: supportability
>         Attachments: HBASE-21312-master-001.patch, HBASE-21312-master-002.patch
>
>
> Normally, Replication needs peer cluster ID to be different from the source. However, if target carries the same cluster ID as source, then during the ReplicationSource initialization process, following is reported in the RegionServer logs before terminating the ReplicationSource thread.
> {code:java}
> 2018-10-08 10:19:09,155 INFO org.apache.hadoop.hbase.replication.regionserver.ReplicationSource: Closing source 1 because: ClusterId 67318852-e2b7-47a7-a303-f1722cb61a06 is replicating to itself: peerClusterId 67318852-e2b7-47a7-a303-f1722cb61a06 which is not allowed by ReplicationEndpoint:org.apache.hadoop.hbase.replication.regionserver.HBaseInterClusterReplicationEndpoint{code}
> Here is the related code snippet which does this in the ReplicationSource class
> {code:java}
> // In rare case, zookeeper setting may be messed up. That leads to the incorrect
>  // peerClusterId value, which is the same as the source clusterId
>  if (clusterId.equals(peerClusterId) && !replicationEndpoint.canReplicateToSameCluster()) {
>  this.terminate("ClusterId " + clusterId + " is replicating to itself: peerClusterId "
>  + peerClusterId + " which is not allowed by ReplicationEndpoint:"
>  + replicationEndpoint.getClass().getName(), null, false);
>  this.manager.removeSource(this);
>  return;
>  }{code}
> It would be good to have this logged at ERROR level instead of INFO for the following reasons
> 1. Under normal situation/case, the Peer Cluster ID would be different
> 2. It would help to easily troubleshoot issues that arises due to matching replication Peer cluster ID



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)