You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "yl09099 (via GitHub)" <gi...@apache.org> on 2023/06/20 12:15:39 UTC
[GitHub] [incubator-uniffle] yl09099 opened a new pull request, #959: [#956][Improvement] Changes the Boolean flag that determines whether a Node is healthy to a state
yl09099 opened a new pull request, #959:
URL: https://github.com/apache/incubator-uniffle/pull/959
### What changes were proposed in this pull request?
Change the ServerNode health status from the original Boolean judgment to the unhealthy state
### Why are the changes needed?
Unhealthy states should not be isolated
Fix: #956
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing unit tests
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236244699
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -78,23 +78,33 @@ public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker
thread.setDaemon(true);
}
- @VisibleForTesting
- void check() {
+
+ public void check() {
for (Checker checker : checkers) {
if (!checker.checkIsHealthy()) {
- isHealthy.set(false);
+ shuffleServer.setServerStatus(ServerStatus.UNHEALTHY);
ShuffleServerMetrics.gaugeIsHealthy.set(1);
return;
}
}
ShuffleServerMetrics.gaugeIsHealthy.set(0);
- isHealthy.set(true);
+ if (shuffleServer.getServerStatus() == ServerStatus.DECOMMISSIONING) {
Review Comment:
why do we assign `DECOMMISSIONING` to it if its origin value is `DECOMMISSIONING`? It seems that we don't change value.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956][Improvement] Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1235185567
##########
proto/src/main/proto/Rss.proto:
##########
@@ -251,7 +251,6 @@ message ShuffleServerHeartBeatRequest {
int64 availableMemory = 4;
int32 eventNumInFlush = 5;
repeated string tags = 6;
- google.protobuf.BoolValue isHealthy = 7;
Review Comment:
Don't modify the field. You can add a comment `// deprecated`. If you modify this, this will be incompatible feature. Although we don't release 1.0 version, we don't need to guarantee the compatibility, I still hope we reduce the similar breaking 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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] yl09099 commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "yl09099 (via GitHub)" <gi...@apache.org>.
yl09099 commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236250502
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -78,23 +78,33 @@ public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker
thread.setDaemon(true);
}
- @VisibleForTesting
- void check() {
+
+ public void check() {
for (Checker checker : checkers) {
if (!checker.checkIsHealthy()) {
- isHealthy.set(false);
+ shuffleServer.setServerStatus(ServerStatus.UNHEALTHY);
ShuffleServerMetrics.gaugeIsHealthy.set(1);
return;
}
}
ShuffleServerMetrics.gaugeIsHealthy.set(0);
- isHealthy.set(true);
+ if (shuffleServer.getServerStatus() == ServerStatus.DECOMMISSIONING) {
Review Comment:
sorry, I made a mistake
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236512958
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -38,14 +39,15 @@ public class HealthCheck {
private static final Logger LOG = LoggerFactory.getLogger(HealthCheck.class);
- private final AtomicBoolean isHealthy;
+ private AtomicReference<ServerStatus> serverStatus;
private final long checkIntervalMs;
private final Thread thread;
private volatile boolean isStop = false;
private List<Checker> checkers = Lists.newArrayList();
- public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker> buildInCheckers) {
- this.isHealthy = isHealthy;
+ public HealthCheck(AtomicReference<ServerStatus> serverStatus,
Review Comment:
Could we use style consistent with other places?
```
public HealthCheck(
AtomicReference<ServerStatus> serverStatus,
ShuffleServerConf conf,
List<Checker> buildInCheckers) {
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi merged pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi merged PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236693528
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -38,14 +39,15 @@ public class HealthCheck {
private static final Logger LOG = LoggerFactory.getLogger(HealthCheck.class);
- private final AtomicBoolean isHealthy;
+ private AtomicReference<ServerStatus> serverStatus;
private final long checkIntervalMs;
private final Thread thread;
private volatile boolean isStop = false;
private List<Checker> checkers = Lists.newArrayList();
- public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker> buildInCheckers) {
- this.isHealthy = isHealthy;
+ public HealthCheck(AtomicReference<ServerStatus> serverStatus,
Review Comment:
@yl09099 Could you address this comment?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1235194567
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -78,23 +78,33 @@ public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker
thread.setDaemon(true);
}
- @VisibleForTesting
- void check() {
+
+ public void check() {
for (Checker checker : checkers) {
if (!checker.checkIsHealthy()) {
- isHealthy.set(false);
+ shuffleServer.setServerStatus(ServerStatus.UNHEALTHY);
ShuffleServerMetrics.gaugeIsHealthy.set(1);
return;
}
}
ShuffleServerMetrics.gaugeIsHealthy.set(0);
- isHealthy.set(true);
+ if (shuffleServer.getServerStatus() == ServerStatus.DECOMMISSIONING) {
Review Comment:
Why do we need 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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236695165
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -38,14 +39,15 @@ public class HealthCheck {
private static final Logger LOG = LoggerFactory.getLogger(HealthCheck.class);
- private final AtomicBoolean isHealthy;
+ private AtomicReference<ServerStatus> serverStatus;
private final long checkIntervalMs;
private final Thread thread;
private volatile boolean isStop = false;
private List<Checker> checkers = Lists.newArrayList();
- public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker> buildInCheckers) {
- this.isHealthy = isHealthy;
+ public HealthCheck(AtomicReference<ServerStatus> serverStatus,
Review Comment:
This indent should be 4.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] yl09099 commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "yl09099 (via GitHub)" <gi...@apache.org>.
yl09099 commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236783812
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -38,14 +39,15 @@ public class HealthCheck {
private static final Logger LOG = LoggerFactory.getLogger(HealthCheck.class);
- private final AtomicBoolean isHealthy;
+ private AtomicReference<ServerStatus> serverStatus;
private final long checkIntervalMs;
private final Thread thread;
private volatile boolean isStop = false;
private List<Checker> checkers = Lists.newArrayList();
- public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker> buildInCheckers) {
- this.isHealthy = isHealthy;
+ public HealthCheck(AtomicReference<ServerStatus> serverStatus,
Review Comment:
sorry, Have been changed
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] yl09099 commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "yl09099 (via GitHub)" <gi...@apache.org>.
yl09099 commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236222475
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -78,23 +78,33 @@ public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker
thread.setDaemon(true);
}
- @VisibleForTesting
- void check() {
+
+ public void check() {
for (Checker checker : checkers) {
if (!checker.checkIsHealthy()) {
- isHealthy.set(false);
+ shuffleServer.setServerStatus(ServerStatus.UNHEALTHY);
ShuffleServerMetrics.gaugeIsHealthy.set(1);
return;
}
}
ShuffleServerMetrics.gaugeIsHealthy.set(0);
- isHealthy.set(true);
+ if (shuffleServer.getServerStatus() == ServerStatus.DECOMMISSIONING) {
Review Comment:
In ShuffleServer. After decommission is called, there are a large number of application is not complete case, ServerStatus state is DECOMMISSIONING, but we can't change the state of ServerStatus health thread detection
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236512958
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -38,14 +39,15 @@ public class HealthCheck {
private static final Logger LOG = LoggerFactory.getLogger(HealthCheck.class);
- private final AtomicBoolean isHealthy;
+ private AtomicReference<ServerStatus> serverStatus;
private final long checkIntervalMs;
private final Thread thread;
private volatile boolean isStop = false;
private List<Checker> checkers = Lists.newArrayList();
- public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker> buildInCheckers) {
- this.isHealthy = isHealthy;
+ public HealthCheck(AtomicReference<ServerStatus> serverStatus,
Review Comment:
Could we use style consistent with other places?
```
public HealthCheck(
AtomicReference<ServerStatus> serverStatus,
ShuffleServerConf conf,
List<Checker> buildInCheckers) {
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956][Improvement] Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1235185567
##########
proto/src/main/proto/Rss.proto:
##########
@@ -251,7 +251,6 @@ message ShuffleServerHeartBeatRequest {
int64 availableMemory = 4;
int32 eventNumInFlush = 5;
repeated string tags = 6;
- google.protobuf.BoolValue isHealthy = 7;
Review Comment:
Don't modify the field. You can add a comment `// deprecated`. If you modify this, this will be incompatible feature. Although we don't release 1.0 version, we don't need guarantee the compatibility, I still hope we reduce the similar breaking 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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236512958
##########
server/src/main/java/org/apache/uniffle/server/HealthCheck.java:
##########
@@ -38,14 +39,15 @@ public class HealthCheck {
private static final Logger LOG = LoggerFactory.getLogger(HealthCheck.class);
- private final AtomicBoolean isHealthy;
+ private AtomicReference<ServerStatus> serverStatus;
private final long checkIntervalMs;
private final Thread thread;
private volatile boolean isStop = false;
private List<Checker> checkers = Lists.newArrayList();
- public HealthCheck(AtomicBoolean isHealthy, ShuffleServerConf conf, List<Checker> buildInCheckers) {
- this.isHealthy = isHealthy;
+ public HealthCheck(AtomicReference<ServerStatus> serverStatus,
Review Comment:
Could we use style consistent with other places?
```
public HealthCheck(
AtomicReference<ServerStatus> serverStatus,
ShuffleServerConf conf,
List<Checker> buildInCheckers) {
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1235462156
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServer.java:
##########
@@ -226,7 +226,7 @@ private void initialization() throws Exception {
if (healthCheckEnable) {
List<Checker> builtInCheckers = Lists.newArrayList();
builtInCheckers.add(storageManager.getStorageChecker());
- healthCheck = new HealthCheck(isHealthy, shuffleServerConf, builtInCheckers);
+ healthCheck = new HealthCheck(this, shuffleServerConf, builtInCheckers);
Review Comment:
You can pass ServerStatus as parameter. We can use AtomicReference as the type of ServerStatus.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] yl09099 commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "yl09099 (via GitHub)" <gi...@apache.org>.
yl09099 commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236200878
##########
proto/src/main/proto/Rss.proto:
##########
@@ -251,7 +251,6 @@ message ShuffleServerHeartBeatRequest {
int64 availableMemory = 4;
int32 eventNumInFlush = 5;
repeated string tags = 6;
- google.protobuf.BoolValue isHealthy = 7;
Review Comment:
Okay, I'll recover
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236578194
##########
coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorGrpcService.java:
##########
@@ -364,11 +364,19 @@ private void logAssignmentResult(String appId, int shuffleId, PartitionRangeAssi
}
private ServerNode toServerNode(ShuffleServerHeartBeatRequest request) {
+ ServerStatus serverStatus = request.hasStatus() ? ServerStatus.fromProto(request.getStatus()) : ServerStatus.ACTIVE;
boolean isHealthy = true;
if (request.hasIsHealthy()) {
isHealthy = request.getIsHealthy().getValue();
+ /**
+ * Compatible with isHealthy version
Review Comment:
```suggestion
* Compatible with older version
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] yl09099 commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "yl09099 (via GitHub)" <gi...@apache.org>.
yl09099 commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236202261
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServer.java:
##########
@@ -226,7 +226,7 @@ private void initialization() throws Exception {
if (healthCheckEnable) {
List<Checker> builtInCheckers = Lists.newArrayList();
builtInCheckers.add(storageManager.getStorageChecker());
- healthCheck = new HealthCheck(isHealthy, shuffleServerConf, builtInCheckers);
+ healthCheck = new HealthCheck(this, shuffleServerConf, builtInCheckers);
Review Comment:
Let me revise it.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236581719
##########
integration-test/common/src/test/java/org/apache/uniffle/test/HealthCheckTest.java:
##########
@@ -1,38 +1,26 @@
-/*
Review Comment:
Why do we remove the license header?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] yl09099 commented on a diff in pull request #959: [#956] refactor: Changes the Boolean flag that determines whether a Node is healthy to a state
Posted by "yl09099 (via GitHub)" <gi...@apache.org>.
yl09099 commented on code in PR #959:
URL: https://github.com/apache/incubator-uniffle/pull/959#discussion_r1236638488
##########
integration-test/common/src/test/java/org/apache/uniffle/test/HealthCheckTest.java:
##########
@@ -1,38 +1,26 @@
-/*
Review Comment:
I moved this file, forgot to add it, let me add it
--
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.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org