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/01/02 17:44:09 UTC
[GitHub] [hbase] saintstack opened a new pull request #979: HBASE-23632
DeadServer cleanup
saintstack opened a new pull request #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #979:
HBASE-23632 DeadServer cleanup
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#discussion_r362617492
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java
##########
@@ -140,12 +107,8 @@ public synchronized void add(ServerName sn) {
*/
public synchronized void notifyServer(ServerName sn) {
boolean added = processingServers.add(sn);
- if (LOG.isDebugEnabled()) {
- if (added) {
- LOG.debug("Added " + sn + "; numProcessing=" + processingServers.size());
- }
- LOG.debug("Started processing " + sn + "; numProcessing=" + processingServers.size());
- }
+ LOG.debug("Started processing {}; numProcessing={}; added={}",
Review comment:
Thanks. You are right. Let me make the logging note better.
Thanks for the review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on a change in pull request #979:
HBASE-23632 DeadServer cleanup
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#discussion_r362568203
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java
##########
@@ -124,14 +94,11 @@ public synchronized boolean areDeadServersInProgress() {
/**
* Adds the server to the dead server list if it's not there already.
*/
- public synchronized void add(ServerName sn) {
+ synchronized void add(ServerName sn) {
if (!deadServers.containsKey(sn)){
deadServers.put(sn, EnvironmentEdgeManager.currentTime());
Review comment:
nit: use putIfAbsent()? Also may be rename method to addIfNotExists() to clarify intent?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #979: HBASE-23632
DeadServer cleanup
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#issuecomment-570353517
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | 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-2.2 Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 41s | branch-2.2 passed |
| +1 :green_heart: | compile | 0m 55s | branch-2.2 passed |
| +1 :green_heart: | checkstyle | 1m 29s | branch-2.2 passed |
| +1 :green_heart: | shadedjars | 4m 21s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | branch-2.2 passed |
| +0 :ok: | spotbugs | 3m 23s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 21s | branch-2.2 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 14s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 27s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 20s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 16m 14s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
| +1 :green_heart: | findbugs | 3m 39s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 151m 43s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 208m 10s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.regionserver.TestHRegionWithInMemoryFlush |
| | hadoop.hbase.quotas.TestClusterScopeQuotaThrottle |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/979 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux ef101f9aad4c 4.15.0-70-generic #79-Ubuntu SMP Tue Nov 12 10:36:11 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-979/out/precommit/personality/provided.sh |
| git revision | branch-2.2 / 3887752b8e |
| Default Java | 1.8.0_181 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/1/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/1/testReport/ |
| Max. process+thread count | 4403 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/1/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| 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
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #979: HBASE-23632
DeadServer cleanup
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#issuecomment-570417694
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | 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 2 new or modified test files. |
||| _ branch-2.2 Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 39s | branch-2.2 passed |
| +1 :green_heart: | compile | 0m 57s | branch-2.2 passed |
| +1 :green_heart: | checkstyle | 1m 31s | branch-2.2 passed |
| +1 :green_heart: | shadedjars | 4m 27s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | branch-2.2 passed |
| +0 :ok: | spotbugs | 3m 48s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 46s | branch-2.2 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 20s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 28s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 24s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 16m 42s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 34s | the patch passed |
| +1 :green_heart: | findbugs | 3m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 145m 1s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 30s | The patch does not generate ASF License warnings. |
| | | 202m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/979 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux f837feec4617 4.15.0-70-generic #79-Ubuntu SMP Tue Nov 12 10:36:11 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-979/out/precommit/personality/provided.sh |
| git revision | branch-2.2 / 3887752b8e |
| Default Java | 1.8.0_181 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/2/testReport/ |
| Max. process+thread count | 4981 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-979/2/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| 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
With regards,
Apache Git Services
[GitHub] [hbase] saintstack merged pull request #979: HBASE-23632 DeadServer
cleanup
Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #979:
HBASE-23632 DeadServer cleanup
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#discussion_r362613772
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java
##########
@@ -124,14 +94,11 @@ public synchronized boolean areDeadServersInProgress() {
/**
* Adds the server to the dead server list if it's not there already.
*/
- public synchronized void add(ServerName sn) {
+ synchronized void add(ServerName sn) {
if (!deadServers.containsKey(sn)){
deadServers.put(sn, EnvironmentEdgeManager.currentTime());
Review comment:
I like this better.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on a change in pull request #979:
HBASE-23632 DeadServer cleanup
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#discussion_r362569794
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java
##########
@@ -140,12 +107,8 @@ public synchronized void add(ServerName sn) {
*/
public synchronized void notifyServer(ServerName sn) {
boolean added = processingServers.add(sn);
- if (LOG.isDebugEnabled()) {
- if (added) {
- LOG.debug("Added " + sn + "; numProcessing=" + processingServers.size());
- }
- LOG.debug("Started processing " + sn + "; numProcessing=" + processingServers.size());
- }
+ LOG.debug("Started processing {}; numProcessing={}; added={}",
Review comment:
nit: Please correct me if I'm wrong. Now that we use notifyServer() as a helper in add(), calling notifyServer() doesn't mean we started processing it? Calling notifyServer() from SCP only means we started processing it, no? My point is log might be a little misleading.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on issue #979: HBASE-23632 DeadServer
cleanup
Posted by GitBox <gi...@apache.org>.
saintstack commented on issue #979: HBASE-23632 DeadServer cleanup
URL: https://github.com/apache/hbase/pull/979#issuecomment-570334045
Address review comments.
----------------------------------------------------------------
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
With regards,
Apache Git Services