You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/15 06:28:57 UTC
[GitHub] [hbase] infraio opened a new pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
infraio opened a new pull request #2071:
URL: https://github.com/apache/hbase/pull/2071
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-661792234
@infraio `TestReplicationAdmin` is consistently failing on branch-2.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-659216624
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 24s | master passed |
| +1 :green_heart: | compile | 0m 55s | master passed |
| +1 :green_heart: | shadedjars | 5m 39s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 29s | the patch passed |
| +1 :green_heart: | compile | 0m 54s | the patch passed |
| +1 :green_heart: | javac | 0m 54s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 36s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 146m 46s | hbase-server in the patch failed. |
| | | 170m 25s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux e34379920a0a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/testReport/ |
| Max. process+thread count | 4644 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio merged pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio merged pull request #2071:
URL: https://github.com/apache/hbase/pull/2071
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455463816
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -501,6 +511,32 @@ private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
}
}
+ private void checkClusterId(String clusterKey) throws DoNotRetryIOException {
+ String peerClusterId = "";
+ try {
+ // Create the peer cluster config for get peer cluster id
+ Configuration peerConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(peerConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
+ } catch (IOException | KeeperException e) {
+ throw new DoNotRetryIOException("Can't get peerClusterId for clusterKey=" + clusterKey, e);
+ }
+ // 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)) {
Review comment:
The peerClusterId was not initialized in the constructor method of ReplicationEndpoint. So ReplicationEndpoint#getPeerUUID() will get null here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-659090238
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 10s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 8s | master passed |
| +1 :green_heart: | checkstyle | 1m 15s | master passed |
| +1 :green_heart: | spotbugs | 2m 9s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 13s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 17s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 16s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 36m 6s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux e7d783199938 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-658588029
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 44s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 49s | master passed |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | spotbugs | 2m 0s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 21s | the patch passed |
| -0 :warning: | checkstyle | 1m 5s | hbase-server: The patch generated 9 new + 114 unchanged - 0 fixed = 123 total (was 114) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 14s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 7s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 32m 49s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux a6e08dad3d03 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455434266
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -493,12 +499,35 @@ private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
}
}
- private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
+ private void checkClusterKeyForHBaseInterClusterReplicationEndpoint(String clusterKey)
+ throws DoNotRetryIOException {
try {
ZKConfig.validateClusterKey(clusterKey);
} catch (IOException e) {
throw new DoNotRetryIOException("Invalid cluster key: " + clusterKey, e);
}
+ String peerClusterId = "";
+ try {
+ Configuration otherConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(otherConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
Review comment:
See the changed code in hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455427867
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -493,12 +499,35 @@ private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
}
}
- private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
+ private void checkClusterKeyForHBaseInterClusterReplicationEndpoint(String clusterKey)
+ throws DoNotRetryIOException {
try {
ZKConfig.validateClusterKey(clusterKey);
} catch (IOException e) {
throw new DoNotRetryIOException("Invalid cluster key: " + clusterKey, e);
}
+ String peerClusterId = "";
+ try {
+ Configuration otherConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(otherConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
Review comment:
Not right. This fetch peer cluster id.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-660856986
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | master passed |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | spotbugs | 1m 59s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 6s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 36s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 8s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 32m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux d942687798df 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9b32943697 |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-659164148
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 8s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 46s | master passed |
| +1 :green_heart: | checkstyle | 1m 13s | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 46s | the patch passed |
| -0 :warning: | checkstyle | 1m 12s | hbase-server: The patch generated 1 new + 114 unchanged - 0 fixed = 115 total (was 114) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 9s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 16s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 35m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 50420a6cf4e1 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-659153641
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 46s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 17s | master passed |
| +1 :green_heart: | compile | 1m 26s | master passed |
| +1 :green_heart: | shadedjars | 7m 46s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 49s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 45s | the patch passed |
| +1 :green_heart: | compile | 1m 33s | the patch passed |
| +1 :green_heart: | javac | 1m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 47s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 228m 50s | hbase-server in the patch failed. |
| | | 263m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c7903439634c 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/testReport/ |
| Max. process+thread count | 2910 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r454901025
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,25 +347,21 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
if (!StringUtils.isBlank(replicationEndpointImpl)) {
- // try creating a instance
- ReplicationEndpoint endpoint;
try {
+ // try creating a instance
endpoint = Class.forName(replicationEndpointImpl)
.asSubclass(ReplicationEndpoint.class).getDeclaredConstructor().newInstance();
} catch (Throwable e) {
throw new DoNotRetryIOException(
"Can not instantiate configured replication endpoint class=" + replicationEndpointImpl,
e);
}
- // do not check cluster key if we are not HBaseInterClusterReplicationEndpoint
- if (!(endpoint instanceof HBaseInterClusterReplicationEndpoint)) {
- checkClusterKey = false;
- }
}
- if (checkClusterKey) {
- checkClusterKey(peerConfig.getClusterKey());
+ // Default is HBaseInterClusterReplicationEndpoint and only it need to check cluster key
+ if (endpoint == null || endpoint instanceof HBaseInterClusterReplicationEndpoint) {
Review comment:
Just a thought while going through this code, not required for the objective of this PR: as mentioned on the code comment, this is specific for HBaseInterClusterReplicationEndpoint, so shouldn't we define a checkClusterKey method on ReplicationEndpoint (with a default implementation), and implement this check specific for HBaseInterClusterReplicationEndpoint in HBaseInterClusterReplicationEndpoint?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-660961831
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 24s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 58s | master passed |
| +1 :green_heart: | compile | 1m 17s | master passed |
| +1 :green_heart: | shadedjars | 6m 40s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 48s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 47s | the patch passed |
| +1 :green_heart: | compile | 1m 12s | the patch passed |
| +1 :green_heart: | javac | 1m 12s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 43s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 217m 41s | hbase-server in the patch passed. |
| | | 248m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6e059752a26e 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9b32943697 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/testReport/ |
| Max. process+thread count | 2903 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-659248670
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 30s | master passed |
| +1 :green_heart: | compile | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 6m 18s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 40s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 28s | the patch passed |
| +1 :green_heart: | compile | 1m 9s | the patch passed |
| +1 :green_heart: | javac | 1m 9s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 21s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 201m 27s | hbase-server in the patch failed. |
| | | 229m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3680f72cba1e 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/testReport/ |
| Max. process+thread count | 3126 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455463412
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,25 +347,21 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
if (!StringUtils.isBlank(replicationEndpointImpl)) {
- // try creating a instance
- ReplicationEndpoint endpoint;
try {
+ // try creating a instance
endpoint = Class.forName(replicationEndpointImpl)
.asSubclass(ReplicationEndpoint.class).getDeclaredConstructor().newInstance();
} catch (Throwable e) {
throw new DoNotRetryIOException(
"Can not instantiate configured replication endpoint class=" + replicationEndpointImpl,
e);
}
- // do not check cluster key if we are not HBaseInterClusterReplicationEndpoint
- if (!(endpoint instanceof HBaseInterClusterReplicationEndpoint)) {
- checkClusterKey = false;
- }
}
- if (checkClusterKey) {
- checkClusterKey(peerConfig.getClusterKey());
+ // Default is HBaseInterClusterReplicationEndpoint and only it need to check cluster key
+ if (endpoint == null || endpoint instanceof HBaseInterClusterReplicationEndpoint) {
Review comment:
For default case, it didn't new a ReplicationEndpoint. There are a check !StringUtils.isBlank(replicationEndpointImpl).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455427724
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -493,12 +499,35 @@ private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
}
}
- private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
+ private void checkClusterKeyForHBaseInterClusterReplicationEndpoint(String clusterKey)
+ throws DoNotRetryIOException {
try {
ZKConfig.validateClusterKey(clusterKey);
} catch (IOException e) {
throw new DoNotRetryIOException("Invalid cluster key: " + clusterKey, e);
}
+ String peerClusterId = "";
Review comment:
No, this fetch the peer cluster id.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-660966111
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 24s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 6m 20s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 3s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 231m 45s | hbase-server in the patch passed. |
| | | 258m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6dddfc415a1b 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9b32943697 |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/testReport/ |
| Max. process+thread count | 2900 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455457366
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,26 +347,26 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
if (!StringUtils.isBlank(replicationEndpointImpl)) {
- // try creating a instance
- ReplicationEndpoint endpoint;
try {
+ // try creating a instance
endpoint = Class.forName(replicationEndpointImpl)
.asSubclass(ReplicationEndpoint.class).getDeclaredConstructor().newInstance();
} catch (Throwable e) {
throw new DoNotRetryIOException(
"Can not instantiate configured replication endpoint class=" + replicationEndpointImpl,
e);
}
- // do not check cluster key if we are not HBaseInterClusterReplicationEndpoint
- if (!(endpoint instanceof HBaseInterClusterReplicationEndpoint)) {
- checkClusterKey = false;
- }
}
- if (checkClusterKey) {
+ // Default is HBaseInterClusterReplicationEndpoint and only it need to check cluster key
+ if (endpoint == null || endpoint instanceof HBaseInterClusterReplicationEndpoint) {
checkClusterKey(peerConfig.getClusterKey());
}
+ // Default is HBaseInterClusterReplicationEndpoint which cannot replicate to same cluster
+ if (endpoint == null || !endpoint.canReplicateToSameCluster()) {
Review comment:
nit: same question as above..null check on endpoint.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -493,12 +499,35 @@ private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
}
}
- private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
+ private void checkClusterKeyForHBaseInterClusterReplicationEndpoint(String clusterKey)
+ throws DoNotRetryIOException {
try {
ZKConfig.validateClusterKey(clusterKey);
} catch (IOException e) {
throw new DoNotRetryIOException("Invalid cluster key: " + clusterKey, e);
}
+ String peerClusterId = "";
Review comment:
oh sorry my bad, I misread the code.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,26 +347,26 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
Review comment:
nit = null is redundant.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -501,6 +511,32 @@ private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
}
}
+ private void checkClusterId(String clusterKey) throws DoNotRetryIOException {
+ String peerClusterId = "";
+ try {
+ // Create the peer cluster config for get peer cluster id
+ Configuration peerConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(peerConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
+ } catch (IOException | KeeperException e) {
+ throw new DoNotRetryIOException("Can't get peerClusterId for clusterKey=" + clusterKey, e);
+ }
Review comment:
We want to close() the zkWatcher? Otherwise there is a connection leak?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -501,6 +511,32 @@ private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
}
}
+ private void checkClusterId(String clusterKey) throws DoNotRetryIOException {
+ String peerClusterId = "";
+ try {
+ // Create the peer cluster config for get peer cluster id
+ Configuration peerConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(peerConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
+ } catch (IOException | KeeperException e) {
+ throw new DoNotRetryIOException("Can't get peerClusterId for clusterKey=" + clusterKey, e);
+ }
+ // 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)) {
Review comment:
Actually, now that I read the code again, why not use ReplicationEndpoint#getPeerUUID().toString(). That wraps the whole logic?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -501,6 +511,32 @@ private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
}
}
+ private void checkClusterId(String clusterKey) throws DoNotRetryIOException {
+ String peerClusterId = "";
+ try {
+ // Create the peer cluster config for get peer cluster id
+ Configuration peerConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(peerConf, this + "check-peer-cluster-key", new Abortable() {
Review comment:
Looks like ZKWatcher is null-safe with abortable. Since we are not actually relying on it, just pass null?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,25 +347,21 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
if (!StringUtils.isBlank(replicationEndpointImpl)) {
- // try creating a instance
- ReplicationEndpoint endpoint;
try {
+ // try creating a instance
endpoint = Class.forName(replicationEndpointImpl)
.asSubclass(ReplicationEndpoint.class).getDeclaredConstructor().newInstance();
} catch (Throwable e) {
throw new DoNotRetryIOException(
"Can not instantiate configured replication endpoint class=" + replicationEndpointImpl,
e);
}
- // do not check cluster key if we are not HBaseInterClusterReplicationEndpoint
- if (!(endpoint instanceof HBaseInterClusterReplicationEndpoint)) {
- checkClusterKey = false;
- }
}
- if (checkClusterKey) {
- checkClusterKey(peerConfig.getClusterKey());
+ // Default is HBaseInterClusterReplicationEndpoint and only it need to check cluster key
+ if (endpoint == null || endpoint instanceof HBaseInterClusterReplicationEndpoint) {
Review comment:
Thanks. Why have endpoint == null check? Ideally it shouldn't be null? (if we are past the try-catch)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-658697131
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 49s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 50s | master passed |
| +1 :green_heart: | compile | 1m 34s | master passed |
| +1 :green_heart: | shadedjars | 8m 1s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 1m 3s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 47s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 35s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 54s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 217m 2s | hbase-server in the patch failed. |
| | | 253m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux aabe8f60bd6a 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/testReport/ |
| Max. process+thread count | 3034 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455430870
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,25 +347,21 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
if (!StringUtils.isBlank(replicationEndpointImpl)) {
- // try creating a instance
- ReplicationEndpoint endpoint;
try {
+ // try creating a instance
endpoint = Class.forName(replicationEndpointImpl)
.asSubclass(ReplicationEndpoint.class).getDeclaredConstructor().newInstance();
} catch (Throwable e) {
throw new DoNotRetryIOException(
"Can not instantiate configured replication endpoint class=" + replicationEndpointImpl,
e);
}
- // do not check cluster key if we are not HBaseInterClusterReplicationEndpoint
- if (!(endpoint instanceof HBaseInterClusterReplicationEndpoint)) {
- checkClusterKey = false;
- }
}
- if (checkClusterKey) {
- checkClusterKey(peerConfig.getClusterKey());
+ // Default is HBaseInterClusterReplicationEndpoint and only it need to check cluster key
+ if (endpoint == null || endpoint instanceof HBaseInterClusterReplicationEndpoint) {
Review comment:
canReplicateToSameCluster() was added onlt for HBaseInterClusterReplicationEndpoint. But checked the code again, it was added to ReplicationEndpoint interface now and it is IA.Replication. So maybe override by user's plugglable ReplicationEndpoiint. Let me update the code to use canReplicateToSameCluster() again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-658660230
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 5m 50s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 39s | master passed |
| +1 :green_heart: | compile | 1m 4s | master passed |
| +1 :green_heart: | shadedjars | 5m 51s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 46s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 53s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 146m 33s | hbase-server in the patch failed. |
| | | 178m 5s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux fa4a13539bd3 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/testReport/ |
| Max. process+thread count | 4504 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455463882
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -501,6 +511,32 @@ private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
}
}
+ private void checkClusterId(String clusterKey) throws DoNotRetryIOException {
+ String peerClusterId = "";
+ try {
+ // Create the peer cluster config for get peer cluster id
+ Configuration peerConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(peerConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
+ } catch (IOException | KeeperException e) {
+ throw new DoNotRetryIOException("Can't get peerClusterId for clusterKey=" + clusterKey, e);
+ }
Review comment:
Good catch. Let me fix this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#discussion_r455409660
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -493,12 +499,35 @@ private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
}
}
- private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
+ private void checkClusterKeyForHBaseInterClusterReplicationEndpoint(String clusterKey)
+ throws DoNotRetryIOException {
try {
ZKConfig.validateClusterKey(clusterKey);
} catch (IOException e) {
throw new DoNotRetryIOException("Invalid cluster key: " + clusterKey, e);
}
+ String peerClusterId = "";
+ try {
+ Configuration otherConf = HBaseConfiguration.createClusterConf(conf, clusterKey);
+ ZKWatcher zkWatcher =
+ new ZKWatcher(otherConf, this + "check-peer-cluster-key", new Abortable() {
+ @Override public void abort(String why, Throwable e) {
+ }
+
+ @Override public boolean isAborted() {
+ return false;
+ }
+ });
+ peerClusterId = ZKClusterId.readClusterIdZNode(zkWatcher);
Review comment:
ReplicationPeerManager is colocated with master, right? you can use master.getClusterId() without doing all this? (that is cached, so saves a ZK round trip).
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -493,12 +499,35 @@ private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
}
}
- private void checkClusterKey(String clusterKey) throws DoNotRetryIOException {
+ private void checkClusterKeyForHBaseInterClusterReplicationEndpoint(String clusterKey)
+ throws DoNotRetryIOException {
try {
ZKConfig.validateClusterKey(clusterKey);
} catch (IOException e) {
throw new DoNotRetryIOException("Invalid cluster key: " + clusterKey, e);
}
+ String peerClusterId = "";
Review comment:
You are fetching the currentClusterId right? Why name it as peerClusterId? I think the argument to this method is peerClusterKey.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java
##########
@@ -337,25 +347,21 @@ public void removeAllQueuesAndHFileRefs(String peerId) throws ReplicationExcepti
private void checkPeerConfig(ReplicationPeerConfig peerConfig) throws DoNotRetryIOException {
String replicationEndpointImpl = peerConfig.getReplicationEndpointImpl();
- boolean checkClusterKey = true;
+ ReplicationEndpoint endpoint = null;
if (!StringUtils.isBlank(replicationEndpointImpl)) {
- // try creating a instance
- ReplicationEndpoint endpoint;
try {
+ // try creating a instance
endpoint = Class.forName(replicationEndpointImpl)
.asSubclass(ReplicationEndpoint.class).getDeclaredConstructor().newInstance();
} catch (Throwable e) {
throw new DoNotRetryIOException(
"Can not instantiate configured replication endpoint class=" + replicationEndpointImpl,
e);
}
- // do not check cluster key if we are not HBaseInterClusterReplicationEndpoint
- if (!(endpoint instanceof HBaseInterClusterReplicationEndpoint)) {
- checkClusterKey = false;
- }
}
- if (checkClusterKey) {
- checkClusterKey(peerConfig.getClusterKey());
+ // Default is HBaseInterClusterReplicationEndpoint and only it need to check cluster key
+ if (endpoint == null || endpoint instanceof HBaseInterClusterReplicationEndpoint) {
Review comment:
Replication#canReplicateToSameCluster() isn't it supposed to do this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2071: HBASE-24743 Reject to add a peer which replicate to itself earlier
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2071:
URL: https://github.com/apache/hbase/pull/2071#issuecomment-659152126
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 47s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 39s | master passed |
| +1 :green_heart: | compile | 1m 9s | master passed |
| +1 :green_heart: | shadedjars | 6m 49s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 44s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 38s | the patch passed |
| +1 :green_heart: | compile | 1m 12s | the patch passed |
| +1 :green_heart: | javac | 1m 12s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 44s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 227m 26s | hbase-server in the patch failed. |
| | | 257m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2071 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5fcf1debfa1f 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2505c7760d |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/testReport/ |
| Max. process+thread count | 3433 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2071/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org