[GitHub] [storm] agresch commented on a change in pull request #3121: STORM-3479 HB timeout configurable on a topology level
agresch commented on a change in pull request #3121: STORM-3479 HB timeout
configurable on a topology level
URL: https://github.com/apache/storm/pull/3121#discussion_r323430617
##
File path:
storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/WorkerLogs.java
##
@@ -189,12 +194,29 @@ public String getTopologyOwnerFromMetadataFile(Path
metaFile) {
*/
public Set getAliveIds(int nowSecs) throws IOException {
return
SupervisorUtils.readWorkerHeartbeats(stormConf).entrySet().stream()
-.filter(entry -> Objects.nonNull(entry.getValue())
-&& !SupervisorUtils.isWorkerHbTimedOut(nowSecs,
entry.getValue(), stormConf))
+.filter(entry -> Objects.nonNull(entry.getValue()) &&
!isTimedOut(nowSecs, entry))
Review comment:
SupervisorUtils .isWorkerHbTimedOut() seems no longer referenced. If it
remains in the code base, we should update to reflect the new topology timeout
parameters. Or else it should be deleted to prevent problems. Seems like it
would be easiest to delete 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3121: STORM-3479 HB timeout configurable on a topology level
agresch commented on a change in pull request #3121: STORM-3479 HB timeout configurable on a topology level URL: https://github.com/apache/storm/pull/3121#discussion_r323396797 ## File path: storm-server/src/main/java/org/apache/storm/DaemonConfig.java ## @@ -479,6 +486,12 @@ @IsStringList public static final String LOGS_GROUPS = "logs.groups"; +/** + * A topology specified timeout for determining which logs are alive. Review comment: topology specific rather than topology specified? topology specific, derived from TOPOLOGY_WORKER_TIMEOUT_SECS if set? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3121: STORM-3479 HB timeout configurable on a topology level
agresch commented on a change in pull request #3121: STORM-3479 HB timeout
configurable on a topology level
URL: https://github.com/apache/storm/pull/3121#discussion_r323398027
##
File path:
storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/WorkerLogs.java
##
@@ -189,12 +194,29 @@ public String getTopologyOwnerFromMetadataFile(Path
metaFile) {
*/
public Set getAliveIds(int nowSecs) throws IOException {
return
SupervisorUtils.readWorkerHeartbeats(stormConf).entrySet().stream()
-.filter(entry -> Objects.nonNull(entry.getValue())
-&& !SupervisorUtils.isWorkerHbTimedOut(nowSecs,
entry.getValue(), stormConf))
+.filter(entry -> Objects.nonNull(entry.getValue()) &&
!isTimedOut(nowSecs, entry))
Review comment:
SupervisorUtils .isWorkerHbTimedOut() - is this no longer valid? Should it
be removed or move the timeout code there?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3121: STORM-3479 HB timeout configurable on a topology level
agresch commented on a change in pull request #3121: STORM-3479 HB timeout configurable on a topology level URL: https://github.com/apache/storm/pull/3121#discussion_r323295909 ## File path: storm-server/src/main/java/org/apache/storm/DaemonConfig.java ## @@ -191,6 +191,13 @@ @IsPositiveNumber public static final String NIMBUS_TASK_TIMEOUT_SECS = "nimbus.task.timeout.secs"; +/** + * When Topology configurable heartbeat timeout enabled, . Review comment: refer to TOPOLOGY_WORKER_TIMEOUT_SECS for clarity 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: us...@infra.apache.org With regards, Apache Git Services
