You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/10/22 09:04:15 UTC
[GitHub] [spark] AngersZhuuuu opened a new pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
AngersZhuuuu opened a new pull request #30131:
URL: https://github.com/apache/spark/pull/30131
### What changes were proposed in this pull request?
For some schedule behavior, we will `scheduleAtFixedRate` to schedule task or heartbeat RPC etc...
If thread blocked by a big fullGc or other reason cause delay, will schedule task repeatedly. For some behavior it's not necessary. We can use `scheduleWithFixedDelay` to avoid it.
### Why are the changes needed?
No
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Not need
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714361615
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734171080
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r511735368
##########
File path: resource-managers/mesos/src/main/scala/org/apache/spark/deploy/mesos/MesosExternalShuffleService.scala
##########
@@ -43,7 +43,7 @@ private[mesos] class MesosExternalBlockHandler(
extends ExternalBlockHandler(transportConf, null) with Logging {
ThreadUtils.newDaemonSingleThreadScheduledExecutor("shuffle-cleaner-watcher")
- .scheduleAtFixedRate(new CleanerThread(), 0, cleanerIntervalS, TimeUnit.SECONDS)
+ .scheduleWithFixedDelay(new CleanerThread(), 0, cleanerIntervalS, TimeUnit.SECONDS)
Review comment:
Similar like make GC from rate to delay.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714541067
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34769/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734096223
retest this please
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716364987
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714425147
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34756/
Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r510075555
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -134,7 +134,7 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
// Periodically revive offers to allow delay scheduling to work
val reviveIntervalMs = conf.get(SCHEDULER_REVIVE_INTERVAL).getOrElse(1000L)
- reviveThread.scheduleAtFixedRate(() => Utils.tryLogNonFatalError {
+ reviveThread.scheduleWithFixedDelay(() => Utils.tryLogNonFatalError {
Option(self).foreach(_.send(ReviveOffers))
Review comment:
> I think it's meaningless to replace for this kind of task whose intentional action happens asynchronically. For example, in this case, sending the message `ReviveOffers` should finish quickly but the intentional action `reviveOffers` may take a longer time. I guess the _delay_ here you want to deal with is from the former one rather than the latter.
If there is a delay in Sender side, avoid send `ReviveOffers` twice, then receive side don't need to call `makeOffers` twice in repeatedly.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r511737571
##########
File path: core/src/main/scala/org/apache/spark/deploy/master/Master.scala
##########
@@ -152,7 +152,7 @@ private[deploy] class Master(
logInfo(s"Spark Master is acting as a reverse proxy. Master, Workers and " +
s"Applications UIs are available at $masterWebUiUrl")
}
- checkForWorkerTimeOutTask = forwardMessageThread.scheduleAtFixedRate(
+ checkForWorkerTimeOutTask = forwardMessageThread.scheduleWithFixedDelay(
() => Utils.tryLogNonFatalError { self.send(CheckForWorkerTimeOut) },
Review comment:
Clean Timeout worker with a delay is ok.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716353391
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34860/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r530711869
##########
File path: core/src/main/scala/org/apache/spark/Heartbeater.scala
##########
@@ -45,7 +45,8 @@ private[spark] class Heartbeater(
val heartbeatTask = new Runnable() {
override def run(): Unit = Utils.logUncaughtExceptions(reportHeartbeat())
}
- heartbeater.scheduleAtFixedRate(heartbeatTask, initialDelay, intervalMs, TimeUnit.MILLISECONDS)
+ heartbeater.scheduleWithFixedDelay(
+ heartbeatTask, initialDelay, intervalMs, TimeUnit.MILLISECONDS)
Review comment:
Done
##########
File path: core/src/main/scala/org/apache/spark/executor/ExecutorMetricsPoller.scala
##########
@@ -99,7 +99,7 @@ private[spark] class ExecutorMetricsPoller(
def start(): Unit = {
poller.foreach { exec =>
val pollingTask: Runnable = () => Utils.logUncaughtExceptions(poll())
- exec.scheduleAtFixedRate(pollingTask, 0L, pollingInterval, TimeUnit.MILLISECONDS)
+ exec.scheduleWithFixedDelay(pollingTask, 0L, pollingInterval, TimeUnit.MILLISECONDS)
}
}
Review comment:
revert 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716351034
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734016301
**[Test build #131820 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131820/testReport)** for PR 30131 at commit [`5643ec2`](https://github.com/apache/spark/commit/5643ec267a339877c1004aae9e1ed8ae1f8f6cca).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714593371
**[Test build #130160 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130160/testReport)** for PR 30131 at commit [`71acc22`](https://github.com/apache/spark/commit/71acc227c7d03d55485681a4f0d99f34b66a0df6).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu edited a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu edited a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-715889934
> if there is heavy gc after sending rpc request, but before task completes - fixed delay will be gc time + wait time vs just wait time.
Double check the code of `ScheduleFutureTask`
```
public void run() {
boolean periodic = isPeriodic();
if (!canRunInCurrentRunState(periodic))
cancel(false);
else if (!periodic)
ScheduledFutureTask.super.run();
else if (ScheduledFutureTask.super.runAndReset()) {
setNextRunTime();
reExecutePeriodic(outerTask);
}
}
```
`resetNextRunTime` called before executing scheduled task. So the real interval between two task should be min(gc time , fixed delay)?
1 gc before `setNextRunTime`, after gc task called then after fixed-delay, next task called
2 gc between `setNextRunTime` and executing task, next execute time setted, if gc time > fixed-delay, next task will be triggered immediately。 then interval between two task is gc time (also reasonable, same as current).
3. gc after executing task, since next run time set, seems no problem.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-740677067
@AngersZhuuuu so what is the user impact you are seeing from this? Is it causing excess load on driver and then things backup more? The context cleaner one makes sense if it ends up calling GC a bunch of times in a row but the default time there is 30min and I wouldn't think you would make that to small anyway so is it really a problem.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714560230
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714454153
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130149/
Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714594601
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714633592
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734097929
**[Test build #131831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131831/testReport)** for PR 30131 at commit [`5643ec2`](https://github.com/apache/spark/commit/5643ec267a339877c1004aae9e1ed8ae1f8f6cca).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734052059
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r530519614
##########
File path: resource-managers/mesos/src/main/java/org/apache/spark/network/shuffle/mesos/MesosExternalBlockStoreClient.java
##########
@@ -89,7 +89,7 @@ private RegisterDriverCallback(TransportClient client, long heartbeatIntervalMs)
@Override
public void onSuccess(ByteBuffer response) {
- heartbeaterThread.scheduleAtFixedRate(
+ heartbeaterThread.scheduleWithFixedDelay(
new Heartbeater(client), 0, heartbeatIntervalMs, TimeUnit.MILLISECONDS);
logger.info("Successfully registered app " + appId + " with external shuffle service.");
Review comment:
I am not sure who is working on mesos to understand impact of this behavior change.
+CC @vanzin, @tgravescs ?
I would prefer preserving behavior if someone cant ack it is fine to modify this.
##########
File path: core/src/main/scala/org/apache/spark/deploy/master/Master.scala
##########
@@ -152,7 +152,7 @@ private[deploy] class Master(
logInfo(s"Spark Master is acting as a reverse proxy. Master, Workers and " +
s"Applications UIs are available at $masterWebUiUrl")
}
- checkForWorkerTimeOutTask = forwardMessageThread.scheduleAtFixedRate(
+ checkForWorkerTimeOutTask = forwardMessageThread.scheduleWithFixedDelay(
() => Utils.tryLogNonFatalError { self.send(CheckForWorkerTimeOut) },
Review comment:
+CC @Ngone51 Any thoughts on how this will impact standalone ? For changes to both `Master` and `Worker` classes.
Typically I would not prefer a behavior change for a cleanup PR.
##########
File path: core/src/main/scala/org/apache/spark/Heartbeater.scala
##########
@@ -45,7 +45,8 @@ private[spark] class Heartbeater(
val heartbeatTask = new Runnable() {
override def run(): Unit = Utils.logUncaughtExceptions(reportHeartbeat())
}
- heartbeater.scheduleAtFixedRate(heartbeatTask, initialDelay, intervalMs, TimeUnit.MILLISECONDS)
+ heartbeater.scheduleWithFixedDelay(
+ heartbeatTask, initialDelay, intervalMs, TimeUnit.MILLISECONDS)
Review comment:
Can we revert this ? Other state piggybacks on heartbeat payload and even if there is jitter, it is better to have a regular/predictable cadence of heartbeat messages.
##########
File path: core/src/main/scala/org/apache/spark/executor/ExecutorMetricsPoller.scala
##########
@@ -99,7 +99,7 @@ private[spark] class ExecutorMetricsPoller(
def start(): Unit = {
poller.foreach { exec =>
val pollingTask: Runnable = () => Utils.logUncaughtExceptions(poll())
- exec.scheduleAtFixedRate(pollingTask, 0L, pollingInterval, TimeUnit.MILLISECONDS)
+ exec.scheduleWithFixedDelay(pollingTask, 0L, pollingInterval, TimeUnit.MILLISECONDS)
}
}
Review comment:
This is documented to be a polling interval, which typically has semantics of rate and not duration between polls.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu edited a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu edited a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716268687
> If we want to take a look at the jdk implementation in case the documentation is unclear, please note that both `scheduleAtFixedRate` and `scheduleWithFixedDelay` rely on `ScheduleFutureTask`.
Yea, I will pay attention to this problem later
> For (1) above, `time` will be set to `current time` + `period` -> and so delay between tasks is `gc delay` + `period`.
`gc delay` + `period` should be interval of last task and current task. Use `scheduleAtFixedRate` is same.
By the way, my fault, in comment https://github.com/apache/spark/pull/30131#issuecomment-715889934, I mean interval of current task and next task.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714399511
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34754/
Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-819931056
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714425109
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34756/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716351034
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714517661
**[Test build #130162 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130162/testReport)** for PR 30131 at commit [`14b39ab`](https://github.com/apache/spark/commit/14b39abbadd4531ddc43095e136978a547ccf02b).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714594617
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130160/
Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714517661
**[Test build #130162 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130162/testReport)** for PR 30131 at commit [`14b39ab`](https://github.com/apache/spark/commit/14b39abbadd4531ddc43095e136978a547ccf02b).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] closed pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #30131:
URL: https://github.com/apache/spark/pull/30131
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734016301
**[Test build #131820 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131820/testReport)** for PR 30131 at commit [`5643ec2`](https://github.com/apache/spark/commit/5643ec267a339877c1004aae9e1ed8ae1f8f6cca).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714631985
**[Test build #130162 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130162/testReport)** for PR 30131 at commit [`14b39ab`](https://github.com/apache/spark/commit/14b39abbadd4531ddc43095e136978a547ccf02b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714550007
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34767/
Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714365710
**[Test build #130149 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130149/testReport)** for PR 30131 at commit [`ceedd55`](https://github.com/apache/spark/commit/ceedd55ed7efc27a3e69a7251f1de46c457971b7).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714502291
**[Test build #130160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130160/testReport)** for PR 30131 at commit [`71acc22`](https://github.com/apache/spark/commit/71acc227c7d03d55485681a4f0d99f34b66a0df6).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tgravescs commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-754182337
ping @AngersZhuuuu
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716077517
If we want to take a look at the jdk implementation in case the documentation is unclear, please note that both `scheduleAtFixedRate` and `scheduleWithFixedDelay` rely on `ScheduleFutureTask` (for the default `ScheduledThreadPoolExecutor` in jdk) - the constructor params are different which change their behavior.
More concretely, take a look at at how `setNextRunTime` computes when it should run next.
For (1) above, `time` will be set to `current time` + `period` -> and so delay between tasks is `gc delay` + `period`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734122042
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714399504
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716364987
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714549995
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714549995
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r510551032
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -134,7 +134,7 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
// Periodically revive offers to allow delay scheduling to work
val reviveIntervalMs = conf.get(SCHEDULER_REVIVE_INTERVAL).getOrElse(1000L)
- reviveThread.scheduleAtFixedRate(() => Utils.tryLogNonFatalError {
+ reviveThread.scheduleWithFixedDelay(() => Utils.tryLogNonFatalError {
Option(self).foreach(_.send(ReviveOffers))
Review comment:
> I think it's meaningless to replace for this kind of task whose intentional action happens asynchronically. For example, in this case, sending the message `ReviveOffers` should finish quickly but the intentional action `reviveOffers` may take a longer time. I guess the _delay_ here you want to deal with is from the former one rather than the latter.
How about retained 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714412774
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34756/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714454146
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734156001
**[Test build #131831 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131831/testReport)** for PR 30131 at commit [`5643ec2`](https://github.com/apache/spark/commit/5643ec267a339877c1004aae9e1ed8ae1f8f6cca).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714502291
**[Test build #130160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130160/testReport)** for PR 30131 at commit [`71acc22`](https://github.com/apache/spark/commit/71acc227c7d03d55485681a4f0d99f34b66a0df6).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716312657
> > If we want to take a look at the jdk implementation in case the documentation is unclear, please note that both `scheduleAtFixedRate` and `scheduleWithFixedDelay` rely on `ScheduleFutureTask`.
>
> Yea, I will pay attention to this problem later
>
> > For (1) above, `time` will be set to `current time` + `period` -> and so delay between tasks is `gc delay` + `period`.
>
> `gc delay` + `period` should be interval of last task and current task. Use `scheduleAtFixedRate` is same.
>
> By the way, my fault, in comment [#30131 (comment)](https://github.com/apache/spark/pull/30131#issuecomment-715889934), I mean interval of current task and next task.
I have detailed on the behavior change if we go from rate to delay, I am still unclear on why we are making all the changes in this PR.
I agree with @Ngone51 that moving periodic gc from rate to delay is good, I am not sure about the rest - can you elaborate why the others are being made ? Any issues resolved/bugs fixed ? If not, limiting to gc change might be good.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714560204
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34769/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714425126
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714361615
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714549939
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34767/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714351982
**[Test build #130147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130147/testReport)** for PR 30131 at commit [`590e2c0`](https://github.com/apache/spark/commit/590e2c07947ef46f892f047e339f41a059b9a34b).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716268687
> If we want to take a look at the jdk implementation in case the documentation is unclear, please note that both `scheduleAtFixedRate` and `scheduleWithFixedDelay` rely on `ScheduleFutureTask`.
Yea, I will pay attention to this problem later
> For (1) above, `time` will be set to `current time` + `period` -> and so delay between tasks is `gc delay` + `period`.
`gc delay` + `period` should be interval of last task and current task. Use `scheduleAtFixedRate` is same.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734034296
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714425126
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734051712
**[Test build #131820 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131820/testReport)** for PR 30131 at commit [`5643ec2`](https://github.com/apache/spark/commit/5643ec267a339877c1004aae9e1ed8ae1f8f6cca).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714365710
**[Test build #130149 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130149/testReport)** for PR 30131 at commit [`ceedd55`](https://github.com/apache/spark/commit/ceedd55ed7efc27a3e69a7251f1de46c457971b7).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-715882017
Are you seeing issues with use of `scheduleAtFixedRate` which are fixed by `scheduleWithFixedDelay` ?
There are nuances in the difference, which can have subtle impact. In the example you mentioned, if there is heavy gc after sending rpc request, but before task completes - fixed delay will be gc time + wait time vs just wait time.
While the scenario mentioned in description is equally valid - two events getting fired very close to each other.
If we are not observing issues currently, I would err on side of caution and not change behavior.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716350832
**[Test build #130260 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130260/testReport)** for PR 30131 at commit [`ef6b5b0`](https://github.com/apache/spark/commit/ef6b5b0fa1ec95980f383c9499836b9d354a6995).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734122042
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716351039
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130260/
Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714534442
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34767/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716333959
> I agree with @Ngone51 that moving periodic gc from rate to delay is good, I am not sure about the rest - can you elaborate why the others are being made ? Any issues resolved/bugs fixed ? If not, limiting to gc change might be good.
Not shown is change about RPC, since this change can avoid some unnecessary repeated RPC about `register and heartbeat`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-715889934
> if there is heavy gc after sending rpc request, but before task completes - fixed delay will be gc time + wait time vs just wait time.
Double check the code of `ScheduleFutureTask`
```
public void run() {
boolean periodic = isPeriodic();
if (!canRunInCurrentRunState(periodic))
cancel(false);
else if (!periodic)
ScheduledFutureTask.super.run();
else if (ScheduledFutureTask.super.runAndReset()) {
setNextRunTime();
reExecutePeriodic(outerTask);
}
}
```
`resetNextRunTime` called before executing scheduled task. So the real interval between two task should be max(gd time + fixed delay , fixed delay)?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r511735804
##########
File path: core/src/main/scala/org/apache/spark/executor/ExecutorMetricsPoller.scala
##########
@@ -99,7 +99,7 @@ private[spark] class ExecutorMetricsPoller(
def start(): Unit = {
poller.foreach { exec =>
val pollingTask: Runnable = () => Utils.logUncaughtExceptions(poll())
- exec.scheduleAtFixedRate(pollingTask, 0L, pollingInterval, TimeUnit.MILLISECONDS)
+ exec.scheduleWithFixedDelay(pollingTask, 0L, pollingInterval, TimeUnit.MILLISECONDS)
Review comment:
Similar like make GC from rate to delay. It doesn't make sense to polling twice Immediately.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714633592
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734034296
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714399504
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716335495
**[Test build #130260 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130260/testReport)** for PR 30131 at commit [`ef6b5b0`](https://github.com/apache/spark/commit/ef6b5b0fa1ec95980f383c9499836b9d354a6995).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r511735253
##########
File path: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala
##########
@@ -447,13 +447,13 @@ private[deploy] class Worker(
logInfo(s"Successfully registered with master $preferredMasterAddress")
registered = true
changeMaster(masterRef, masterWebUiUrl, masterAddress)
- forwardMessageScheduler.scheduleAtFixedRate(
+ forwardMessageScheduler.scheduleWithFixedDelay(
() => Utils.tryLogNonFatalError { self.send(SendHeartbeat) },
0, HEARTBEAT_MILLIS, TimeUnit.MILLISECONDS)
if (CLEANUP_ENABLED) {
logInfo(
s"Worker cleanup enabled; old application directories will be deleted in: $workDir")
- forwardMessageScheduler.scheduleAtFixedRate(
+ forwardMessageScheduler.scheduleWithFixedDelay(
() => Utils.tryLogNonFatalError { self.send(WorkDirCleanup) },
CLEANUP_INTERVAL_MILLIS, CLEANUP_INTERVAL_MILLIS, TimeUnit.MILLISECONDS)
Review comment:
Similar like make GC from rate to delay
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714454146
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r510054542
##########
File path: core/src/main/scala/org/apache/spark/ContextCleaner.scala
##########
@@ -126,7 +126,7 @@ private[spark] class ContextCleaner(
cleaningThread.setDaemon(true)
cleaningThread.setName("Spark Context Cleaner")
cleaningThread.start()
- periodicGCService.scheduleAtFixedRate(() => System.gc(),
+ periodicGCService.scheduleWithFixedDelay(() => System.gc(),
Review comment:
This looks like a reasonable change to me.
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -134,7 +134,7 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
// Periodically revive offers to allow delay scheduling to work
val reviveIntervalMs = conf.get(SCHEDULER_REVIVE_INTERVAL).getOrElse(1000L)
- reviveThread.scheduleAtFixedRate(() => Utils.tryLogNonFatalError {
+ reviveThread.scheduleWithFixedDelay(() => Utils.tryLogNonFatalError {
Option(self).foreach(_.send(ReviveOffers))
Review comment:
I think it's meaningless to replace for this kind of task whose intentional action happens asynchronically. For example, in this case, sending the message `ReviveOffers` should finish quickly but the intentional action `reviveOffers` may take a longer time. I guess the *delay* here you want to deal with is from the former one rather than the latter.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r510075555
##########
File path: core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala
##########
@@ -134,7 +134,7 @@ class CoarseGrainedSchedulerBackend(scheduler: TaskSchedulerImpl, val rpcEnv: Rp
// Periodically revive offers to allow delay scheduling to work
val reviveIntervalMs = conf.get(SCHEDULER_REVIVE_INTERVAL).getOrElse(1000L)
- reviveThread.scheduleAtFixedRate(() => Utils.tryLogNonFatalError {
+ reviveThread.scheduleWithFixedDelay(() => Utils.tryLogNonFatalError {
Option(self).foreach(_.send(ReviveOffers))
Review comment:
> I think it's meaningless to replace for this kind of task whose intentional action happens asynchronically. For example, in this case, sending the message `ReviveOffers` should finish quickly but the intentional action `reviveOffers` may take a longer time. I guess the _delay_ here you want to deal with is from the former one rather than the latter.
If there is a delay in Sender side, avoid send `ReviveOffers` twice, then receive side don't need to call `makeOffers` twice in repeatedly.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716335495
**[Test build #130260 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130260/testReport)** for PR 30131 at commit [`ef6b5b0`](https://github.com/apache/spark/commit/ef6b5b0fa1ec95980f383c9499836b9d354a6995).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716804124
+CC @otterc
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714453548
**[Test build #130149 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130149/testReport)** for PR 30131 at commit [`ceedd55`](https://github.com/apache/spark/commit/ceedd55ed7efc27a3e69a7251f1de46c457971b7).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r511736138
##########
File path: core/src/main/scala/org/apache/spark/deploy/Client.scala
##########
@@ -119,7 +119,7 @@ private class ClientEndpoint(
asyncSendToMasterAndForwardReply[KillDriverResponse](RequestKillDriver(driverId))
}
logInfo("... waiting before polling master for driver state")
- forwardMessageThread.scheduleAtFixedRate(() => Utils.tryLogNonFatalError {
+ forwardMessageThread.scheduleWithFixedDelay(() => Utils.tryLogNonFatalError {
monitorDriverStatus()
}, 5000, REPORT_DRIVER_STATUS_INTERVAL, TimeUnit.MILLISECONDS)
Review comment:
Async send message, remove this 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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-716364968
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34860/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714361572
**[Test build #130147 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130147/testReport)** for PR 30131 at commit [`590e2c0`](https://github.com/apache/spark/commit/590e2c07947ef46f892f047e339f41a059b9a34b).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a change in pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30131:
URL: https://github.com/apache/spark/pull/30131#discussion_r511739920
##########
File path: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala
##########
@@ -103,7 +103,7 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
private val killExecutorThread = ThreadUtils.newDaemonSingleThreadExecutor("kill-executor-thread")
override def onStart(): Unit = {
- timeoutCheckingTask = eventLoopThread.scheduleAtFixedRate(
+ timeoutCheckingTask = eventLoopThread.scheduleWithFixedDelay(
() => Utils.tryLogNonFatalError { Option(self).foreach(_.ask[Boolean](ExpireDeadHosts)) },
Review comment:
call for clean dead host, also just need with delay.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714351982
**[Test build #130147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130147/testReport)** for PR 30131 at commit [`590e2c0`](https://github.com/apache/spark/commit/590e2c07947ef46f892f047e339f41a059b9a34b).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714594601
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30131: [WIP][SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-714560230
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30131: [SPARK-33220][CORE]Use `scheduleWithFixedDelay` to avoid repeated unnecessary scheduling for a short time
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30131:
URL: https://github.com/apache/spark/pull/30131#issuecomment-734097929
**[Test build #131831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131831/testReport)** for PR 30131 at commit [`5643ec2`](https://github.com/apache/spark/commit/5643ec267a339877c1004aae9e1ed8ae1f8f6cca).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org