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/06/09 18:42:12 UTC
[GitHub] [hbase] apurtell opened a new pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
apurtell opened a new pull request #1879:
URL: https://github.com/apache/hbase/pull/1879
----------------------------------------------------------------
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] apurtell commented on pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#issuecomment-641500736
Tests pass. Same result with `-Dzookeeper.version=3.6.0` and `-Dzookeeper.version=3.4.13`
[INFO] -------------------------------------------------------
[INFO] T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZooKeeperMainServer
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.158 s - in org.apache.hadoop.hbase.zookeeper.TestZooKeeperMainServer
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestRecoverableZooKeeper
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.794 s - in org.apache.hadoop.hbase.zookeeper.TestRecoverableZooKeeper
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZooKeeperNodeTracker
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.453 s - in org.apache.hadoop.hbase.zookeeper.TestZooKeeperNodeTracker
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZKMulti
[INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.168 s - in org.apache.hadoop.hbase.zookeeper.TestZKMulti
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZooKeeperACL
[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 9.977 s - in org.apache.hadoop.hbase.zookeeper.TestZooKeeperACL
[INFO] Running org.apache.hadoop.hbase.zookeeper.lock.TestZKInterProcessReadWriteLock
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 25.908 s - in org.apache.hadoop.hbase.zookeeper.lock.TestZKInterProcessReadWriteLock
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZKLeaderManager
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.006 s - in org.apache.hadoop.hbase.zookeeper.TestZKLeaderManager
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestHQuorumPeer
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.587 s - in org.apache.hadoop.hbase.zookeeper.TestHQuorumPeer
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZKTableStateManager
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.122 s - in org.apache.hadoop.hbase.zookeeper.TestZKTableStateManager
[INFO] Running org.apache.hadoop.hbase.zookeeper.TestZKAuthFailedRecovery
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 29.037 s - in org.apache.hadoop.hbase.zookeeper.TestZKAuthFailedRecovery
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 42, Failures: 0, Errors: 0, Skipped: 0
----------------------------------------------------------------
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] apurtell commented on pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#issuecomment-641601299
@bharathv I attempted to address your feedback with comments. Please let me know if this works for you or if more should be done.
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437738544
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestHQuorumPeer.java
##########
@@ -111,25 +115,39 @@
QuorumPeerConfig config = new QuorumPeerConfig();
config.parseProperties(properties);
- assertEquals(this.dataDir.toString(), config.getDataDir());
+ assertEquals(this.dataDir.toString(), config.getDataDir().toString());
assertEquals(2181, config.getClientPortAddress().getPort());
Map<Long,QuorumServer> servers = config.getServers();
assertEquals(3, servers.size());
assertTrue(servers.containsKey(Long.valueOf(0)));
QuorumServer server = servers.get(Long.valueOf(0));
- assertEquals("localhost", server.addr.getHostName());
+ assertEquals("localhost", getHostName(server));
// Override with system property.
System.setProperty("hbase.master.hostname", "foo.bar");
is = new ByteArrayInputStream(s.getBytes());
properties = ZKConfig.parseZooCfg(conf, is);
assertEquals("foo.bar:2888:3888", properties.get("server.0"));
-
config.parseProperties(properties);
servers = config.getServers();
server = servers.get(Long.valueOf(0));
- assertEquals("foo.bar", server.addr.getHostName());
+ assertEquals("foo.bar", getHostName(server));
+ }
+
+ private static String getHostName(QuorumServer server) throws Exception {
+ String hostname;
+ switch (server.addr.getClass().getName()) {
Review comment:
I can add a comment. I think the cross-version issues are clear enough by this resort to reflection :-(
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437737134
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
The current signature proposes we throw IOException or InterruptedException. ZK 3.6 throws another type of checked exception, which causes a compilation error. Therefore I catch that and potentially others, and wrap it into an IOE.
----------------------------------------------------------------
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 #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437747899
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
> ZK 3.6 throws another type of checked exception, which causes a compilation error
I didn't see it here [1], hence my question. May be I missed something.
[1] https://github.com/apache/zookeeper/blob/branch-3.6/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java#L380
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java
##########
@@ -85,14 +85,20 @@ public static void main(String[] args) {
}
private static void runZKServer(QuorumPeerConfig zkConfig) throws UnknownHostException, IOException {
Review comment:
Ah, okay. I missed the AdminServerException exception from the signature, wfm to keep it as-is.
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437643117
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
##########
@@ -1811,9 +1811,10 @@ private static Op toZooKeeperOp(ZooKeeperWatcher zkw, ZKUtilOp op)
*/
public static void multiOrSequential(ZooKeeperWatcher zkw, List<ZKUtilOp> ops,
boolean runSequentialOnMultiFailure) throws KeeperException {
- if (ops == null) return;
+ if (ops == null || ops.isEmpty()) {
Review comment:
ZooKeeper 3.6 throws NPE if the multi list is empty
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437737134
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
The current signature proposes we throw IOException or InterruptedException. ZK 3.6 throws another type of checked exception, which causes a compilation error. Therefore I catch that and potentially others, and wrap it into an IOE.
Adding the new checked exception to the signature would cause a compilation problem with 3.4, so that is not possible due to this legacy.
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java
##########
@@ -85,14 +85,20 @@ public static void main(String[] args) {
}
private static void runZKServer(QuorumPeerConfig zkConfig) throws UnknownHostException, IOException {
Review comment:
The current signature proposes we throw IOException or InterruptedException. ZK 3.6 throws another type of checked exception, which causes a compilation error. Therefore I catch that and potentially others, and wrap it into an IOE.
Adding the new checked exception to the signature would cause a compilation problem with 3.4, so that is not possible due to this legacy.
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437738171
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java
##########
@@ -85,14 +85,20 @@ public static void main(String[] args) {
}
private static void runZKServer(QuorumPeerConfig zkConfig) throws UnknownHostException, IOException {
Review comment:
> UnknownHostException is also an IOE
Might be true but the method signature here was what it was and there is no need to fix it. I have little opinion either way other than a preference to minimize change.
----------------------------------------------------------------
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 #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#issuecomment-641601238
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 7m 14s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +0 :ok: | mvndep | 2m 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 8m 0s | branch-1 passed |
| +1 :green_heart: | compile | 0m 59s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 1m 9s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 2m 16s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 1s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 1m 7s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 2m 39s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 14s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 58s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 8s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 1m 8s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 35s | hbase-client: The patch generated 0 new + 62 unchanged - 1 fixed = 62 total (was 63) |
| -1 :x: | checkstyle | 1m 37s | hbase-server: The patch generated 1 new + 14 unchanged - 0 fixed = 15 total (was 14) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 48s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 36s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 50s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 1m 7s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 4m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 41s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 126m 50s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 57s | The patch does not generate ASF License warnings. |
| | | 186m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1879 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 0b95a1f29939 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1879/out/precommit/personality/provided.sh |
| git revision | branch-1 / b6598cc |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/1/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/1/testReport/ |
| Max. process+thread count | 4485 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/1/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| 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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437737505
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java
##########
@@ -85,14 +85,20 @@ public static void main(String[] args) {
}
private static void runZKServer(QuorumPeerConfig zkConfig) throws UnknownHostException, IOException {
Review comment:
The current signature proposes we throw IOException or UnknownHostException. ZK 3.6 throws another type of checked exception, which causes a compilation error. Therefore I catch that and potentially others, and wrap it into an IOE.
Adding the new checked exception to the signature would cause a compilation problem with 3.4, so that is not possible due to this legacy.
----------------------------------------------------------------
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 #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437846013
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
Looks like the root cause is this https://issues.apache.org/jira/browse/ZOOKEEPER-3760. I was looking in the branch-3.6 but the release bits of 3.6.0 didn't include this patch.
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437643393
##########
File path: hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon
##########
@@ -294,7 +294,7 @@ AssignmentManager assignmentManager = master.getAssignmentManager();
</tr>
<tr>
<td>ZooKeeper Client Version</td>
- <td><% org.apache.zookeeper.Version.getVersion() %>, revision=<% org.apache.zookeeper.Version.getRevision() %></td>
+ <td><% org.apache.zookeeper.Version.getVersion() %></td>
Review comment:
3.6.0 removed getRevision; 3.6.1 puts it back. Do we need it? I think: no
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437788463
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project hbase-server: Compilation failure
[ERROR] /Users/apurtell/src/hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java:[70,17] unreported exception org.apache.zookeeper.cli.CliException; must be caught or declared to be thrown
----------------------------------------------------------------
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 #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437681253
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java
##########
@@ -85,14 +85,20 @@ public static void main(String[] args) {
}
private static void runZKServer(QuorumPeerConfig zkConfig) throws UnknownHostException, IOException {
Review comment:
UnknownHostException is also an IOE, so I think you can just remove UnknownHostException from the method signature and all is good? no need to try/catch/wrap blocks?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
Why this? Any un-checked exception is propagated as-is?
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestHQuorumPeer.java
##########
@@ -111,25 +115,39 @@
QuorumPeerConfig config = new QuorumPeerConfig();
config.parseProperties(properties);
- assertEquals(this.dataDir.toString(), config.getDataDir());
+ assertEquals(this.dataDir.toString(), config.getDataDir().toString());
assertEquals(2181, config.getClientPortAddress().getPort());
Map<Long,QuorumServer> servers = config.getServers();
assertEquals(3, servers.size());
assertTrue(servers.containsKey(Long.valueOf(0)));
QuorumServer server = servers.get(Long.valueOf(0));
- assertEquals("localhost", server.addr.getHostName());
+ assertEquals("localhost", getHostName(server));
// Override with system property.
System.setProperty("hbase.master.hostname", "foo.bar");
is = new ByteArrayInputStream(s.getBytes());
properties = ZKConfig.parseZooCfg(conf, is);
assertEquals("foo.bar:2888:3888", properties.get("server.0"));
-
config.parseProperties(properties);
servers = config.getServers();
server = servers.get(Long.valueOf(0));
- assertEquals("foo.bar", server.addr.getHostName());
+ assertEquals("foo.bar", getHostName(server));
+ }
+
+ private static String getHostName(QuorumServer server) throws Exception {
+ String hostname;
+ switch (server.addr.getClass().getName()) {
Review comment:
nit: I think this for cross-version compatibility, a quick comment would be nice.
----------------------------------------------------------------
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] apurtell commented on pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#issuecomment-641601976
Another update to fix checkstyle nits.
----------------------------------------------------------------
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] apurtell commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r437787274
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperMainServer.java
##########
@@ -66,8 +66,14 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args)
* @throws IOException
* @throws InterruptedException
*/
- void runCmdLine() throws KeeperException, IOException, InterruptedException {
- processCmd(this.cl);
+ void runCmdLine() throws IOException, InterruptedException {
+ try {
+ processCmd(this.cl);
+ } catch (IOException | InterruptedException e) {
+ throw e;
+ } catch (Exception e) {
Review comment:
This is a port of internal work, let me remove this hunk and check 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] virajjasani commented on a change in pull request #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#discussion_r438262193
##########
File path: hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon
##########
@@ -294,7 +294,7 @@ AssignmentManager assignmentManager = master.getAssignmentManager();
</tr>
<tr>
<td>ZooKeeper Client Version</td>
- <td><% org.apache.zookeeper.Version.getVersion() %>, revision=<% org.apache.zookeeper.Version.getRevision() %></td>
+ <td><% org.apache.zookeeper.Version.getVersion() %></td>
Review comment:
Agree, no need of revision IMHO
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
##########
@@ -1811,9 +1811,10 @@ private static Op toZooKeeperOp(ZooKeeperWatcher zkw, ZKUtilOp op)
*/
public static void multiOrSequential(ZooKeeperWatcher zkw, List<ZKUtilOp> ops,
boolean runSequentialOnMultiFailure) throws KeeperException {
- if (ops == null) return;
+ if (ops == null || ops.isEmpty()) {
Review comment:
Did our unit tests catch this one or you knew it already? I just want to know what preparation we should do while upgrading ZK or any other heavy dependency.
----------------------------------------------------------------
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 #1879: HBASE-24525 [branch-1] Support ZooKeeper 3.6.0+
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1879:
URL: https://github.com/apache/hbase/pull/1879#issuecomment-641658595
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 54s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +0 :ok: | mvndep | 2m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 8m 0s | branch-1 passed |
| +1 :green_heart: | compile | 1m 1s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 1m 8s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 2m 13s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 1s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 1m 5s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 2m 41s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 16s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 54s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | compile | 1m 9s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 1m 9s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 35s | hbase-client: The patch generated 0 new + 62 unchanged - 1 fixed = 62 total (was 63) |
| +1 :green_heart: | checkstyle | 1m 33s | The patch passed checkstyle in hbase-server |
| -1 :x: | whitespace | 0m 0s | The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | shadedjars | 2m 59s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 35s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 1m 8s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 4m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 40s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 129m 22s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 57s | The patch does not generate ASF License warnings. |
| | | 188m 7s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1879 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 8f82409227b7 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 | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1879/out/precommit/personality/provided.sh |
| git revision | branch-1 / b6598cc |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| whitespace | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/2/artifact/out/whitespace-eol.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/2/testReport/ |
| Max. process+thread count | 4053 (vs. ulimit of 10000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1879/2/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| 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