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/07/05 21:08:09 UTC

[GitHub] [spark] sarutak opened a new pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

sarutak opened a new pull request #29002:
URL: https://github.com/apache/spark/pull/29002


   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   This PR changes the order between initialization for ExecutorPlugin and starting heartbeat thread in Executor.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   In the current master, heartbeat thread in a executor starts after plugin initialization so if the initialization takes long time, heartbeat is not sent to driver and the executor will be removed from cluster.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   Yes. Plugins for executors will be allowed to take long time for initialization. 
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   New testcase.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658240267






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663790544


   **[Test build #126520 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126520/testReport)** for PR 29002 at commit [`0b8da96`](https://github.com/apache/spark/commit/0b8da9620e736fc5cb485c349866593033b1fe33).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659083732






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660481487






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-666746857


   Personally I would be fine with just removing the test.  I'm not sure how much benefit it really adds for the time it takes to run the test and if its being flaky I'm guessing you are going to have to increase the times.  


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657958137






----------------------------------------------------------------
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] sarutak commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r453749208



##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,77 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    val tempDir = Utils.createTempDir()

Review comment:
       I'll try to use `withTempDir`.




----------------------------------------------------------------
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] sarutak edited a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656006944


   `ExecutorRegistered` is regarded as the first heartbeat message although it's not the real `Heartbeat`. 
   
   https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala#L114
   
   So If we reorder the initialization, it should be placed before the registration, not after heartbeater started right?


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655337239


   **[Test build #125316 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125316/testReport)** for PR 29002 at commit [`1ccb1b7`](https://github.com/apache/spark/commit/1ccb1b73f50f3eeb0bd5b05b7d59c6165e841e55).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656717914


   **[Test build #125591 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125591/testReport)** for PR 29002 at commit [`1ccb1b7`](https://github.com/apache/spark/commit/1ccb1b73f50f3eeb0bd5b05b7d59c6165e841e55).
    * 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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659204058


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125932/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654502690


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125110/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657877404


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125789/
   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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658648528


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657916600






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657758781






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665150199


   it looks like arrow tests failing which shouldn't be related, kicked once more


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656574275






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665372255


   It's strange that the reason of the R test failure implies that deprecated function is used. But it's resolved in #29252 .
   I'll rebase this change.
   ```
   2020-07-26T03:10:50.2710895Z test_sparkSQL_arrow.R:39: error: createDataFrame/collect Arrow optimization
   2020-07-26T03:10:50.2711900Z (converted from warning) Use 'read_ipc_stream' or 'read_feather' instead.
   2020-07-26T03:10:50.2712135Z Backtrace:
   2020-07-26T03:10:50.2712459Z   1. base::tryCatch(...) tests/fulltests/test_sparkSQL_arrow.R:39:2
   2020-07-26T03:10:50.2712724Z   7. SparkR::collect(createDataFrame(mtcars))
   2020-07-26T03:10:50.2712988Z   8. SparkR:::.local(x, ...)
   2020-07-26T03:10:50.2713221Z  11. arrow::read_arrow(readRaw(conn))
   2020-07-26T03:10:50.2713946Z  12. base::.Deprecated(msg = "Use 'read_ipc_stream' or 'read_feather' instead.")
   2020-07-26T03:10:50.2714178Z  13. base::warning(...)
   2020-07-26T03:10:50.2714462Z  14. base::withRestarts(...)
   2020-07-26T03:10:50.2714735Z  15. base:::withOneRestart(expr, restarts[[1L]])
   2020-07-26T03:10:50.2714999Z  16. base:::doWithOneRestart(return(expr), restart)
   ```


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657758781






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653959103


   **[Test build #124964 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124964/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).
    * This patch **fails to generate documentation**.
    * 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] tgravescs commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r458816016



##########
File path: core/src/main/scala/org/apache/spark/executor/Executor.scala
##########
@@ -227,6 +222,13 @@ private[spark] class Executor(
 
   metricsPoller.start()

Review comment:
       is there a reason we put this after metrics poller as well? I think I would rather init happen as soon as possible. Some people were using this for metrics initially.




----------------------------------------------------------------
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] sarutak edited a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656006944


   `ExecutorRegistered` is regarded as the first heartbeat message although it's not the real `Heartbeat`. 
   
   https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala#L114
   
   So If we reorder the initialization, it should be placed before the registration, not after heartbeater started.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653971628


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124993/
   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] Ngone51 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658683203


   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] SparkQA removed a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653980498


   **[Test build #124999 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124999/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665674505


   finally, tests pass, merged to master and branch-3.0.  thanks @sarutak 


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972321


   **[Test build #124994 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124994/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657916150


   **[Test build #125798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125798/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663821652






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657761388


   **[Test build #125789 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125789/testReport)** for PR 29002 at commit [`dba4c91`](https://github.com/apache/spark/commit/dba4c91fedba50ead3df311f440b4c9c4d970242).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659709380


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660464379


   **[Test build #126105 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126105/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659083732






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659083037


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665373210






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665372757






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663821524


   **[Test build #126533 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126533/testReport)** for PR 29002 at commit [`0b8da96`](https://github.com/apache/spark/commit/0b8da9620e736fc5cb485c349866593033b1fe33).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663802208


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126520/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659204047


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653970845


   **[Test build #124993 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124993/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653969554






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657916600






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657958140


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125798/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663835250






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663821524


   **[Test build #126533 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126533/testReport)** for PR 29002 at commit [`0b8da96`](https://github.com/apache/spark/commit/0b8da9620e736fc5cb485c349866593033b1fe33).


----------------------------------------------------------------
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] sarutak commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r453748409



##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,77 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    val tempDir = Utils.createTempDir()
+
+    val importStatements =
+      """
+        |import java.util.Map;
+        |import org.apache.spark.api.plugin.*;

Review comment:
       Yeah, it's better.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659020113


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665372757


   **[Test build #126742 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126742/testReport)** for PR 29002 at commit [`f106fe9`](https://github.com/apache/spark/commit/f106fe967d6c303b220d5f36768553219b69c09a).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655337806






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653939712


   **[Test build #124964 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124964/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659142838


   **[Test build #125932 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125932/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653939727






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654382697


   **[Test build #125110 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125110/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656725013






----------------------------------------------------------------
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] sarutak edited a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656247590


   > Ideally, that way should work as well. However, it seems plugin initialization relies on some internal instances of Executor. So, if > we want to initialize plugin out of the executor, I'm afraid we need more changes.
   
   Yes, if we reorder the initialization with another way, the initialization would be put on executor backends like `CoarseGrainedExecutorBackend` and we might need more changes as you are afraid.
   
   > BTW, I think your current implementation can already work as I mentioned above.
   
   Yeah, I misunderstood. `LaunchExecutor` is sent after `Executor` is instansiated so the current implementation works.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663557958


   sorry had stuff come up internally, I'll look more today.


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653979708


   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] SparkQA commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654054383


   **[Test build #124999 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124999/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).
    * 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] maropu commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660471871


   Seems like all the tests passed in GitHub Actions?


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658860191






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663821193


   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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658150610






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657633060


   @Ngone51 Thanks for the feedback. I'll fix them.


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-662134570


   cc: @vanzin @tgravescs 


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656006944


   `ExecutorRegistered` is regarded as the first heartbeat message although it's not the real `Heartbeat`. So If we reorder the initialization, it should be placed before the registration, not after heartbeater started.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655337239


   **[Test build #125316 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125316/testReport)** for PR 29002 at commit [`1ccb1b7`](https://github.com/apache/spark/commit/1ccb1b73f50f3eeb0bd5b05b7d59c6165e841e55).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653959474


   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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658150271


   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] sarutak commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r460346359



##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,73 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    withTempDir { tempDir =>
+      val sparkPluginCodeBody =
+        """
+          |@Override
+          |public org.apache.spark.api.plugin.ExecutorPlugin executorPlugin() {
+          |  return new TestExecutorPlugin();
+          |}
+          |
+          |@Override
+          |public org.apache.spark.api.plugin.DriverPlugin driverPlugin() { return null; }
+        """.stripMargin
+      val executorPluginBody =
+        """
+          |@Override
+          |public void init(
+          |    org.apache.spark.api.plugin.PluginContext ctx,
+          |    java.util.Map<String, String> extraConf) {
+          |  try {
+          |    Thread.sleep(30 * 1000);
+          |  } catch (InterruptedException e) {
+          |    throw new RuntimeException(e);
+          |  }
+          |}
+        """.stripMargin
+
+      val compiledExecutorPlugin = TestUtils.createCompiledClass(
+        "TestExecutorPlugin",
+        tempDir,
+        "",
+        null,
+        Seq.empty,
+        Seq("org.apache.spark.api.plugin.ExecutorPlugin"),
+        executorPluginBody)
+
+      val thisClassPath =
+        sys.props("java.class.path").split(File.pathSeparator).map(p => new File(p).toURI.toURL)
+      val compiledSparkPlugin = TestUtils.createCompiledClass(
+        "TestSparkPlugin",
+        tempDir,
+        "",
+        null,
+        Seq(tempDir.toURI.toURL) ++ thisClassPath,
+        Seq("org.apache.spark.api.plugin.SparkPlugin"),
+        sparkPluginCodeBody)
+
+      val jarUrl = TestUtils.createJar(
+        Seq(compiledSparkPlugin, compiledExecutorPlugin),
+        new File(tempDir, "testPlugin.jar"))
+
+      val unusedJar = TestUtils.createJarWithClasses(Seq.empty)
+      val args = Seq(
+        "--class", SimpleApplicationTest.getClass.getName.stripSuffix("$"),
+        "--name", "testApp",
+        "--master", "local-cluster[1,1,1024]",
+        "--conf", "spark.plugins=TestSparkPlugin",
+        "--conf", "spark.storage.blockManagerSlaveTimeoutMs=" + 10 * 1000,
+        "--conf", "spark.network.timeoutInterval=" + 10 * 1000,
+        "--conf", "spark.executor.heartbeatInterval=" + 5 * 1000,
+        "--conf", "spark.executor.extraClassPath=" + jarUrl.toString,
+        "--conf", "spark.driver.extraClassPath=" + jarUrl.toString,
+        "--conf", "spark.ui.enabled=false",
+        unusedJar.toString)
+      SparkSubmitSuite.runSparkSubmit(args, timeout = 2.minutes)

Review comment:
       8 seconds works on my laptop so let's go with the number. 




----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660481487






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654501763


   **[Test build #125110 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125110/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).
    * 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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655696048


   @mridulm 
   >As soon as we heartbeat to driver, we will start getting task assignment, etc - how does the interact with a plugin which has not yet completed initialization ?
   
   Yeah that's right. 
   
   >Instead of changing order, why not document that inordinate delays in plugin initialization should be avoided and deferred to >seperate thread/managed by plugin.
   
   This can be a reasonable compromise. Another solution can be to perform initialization before registering executors and then it's guaranteed that plugins are initialized when tasks start running.
   What do you think?


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658862178


   **[Test build #125895 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125895/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655731079


   > This can be a reasonable compromise. Another solution can be to perform initialization before registering executors and then it's guaranteed that plugins are initialized when tasks start running.
   What do you think?
   
   Isn't that not what is implicitly handling now ? And causing issues if plugin takes a long time to initialize ?


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658153415


   **[Test build #125831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125831/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660464123


   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] Ngone51 commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r453164341



##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,77 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    val tempDir = Utils.createTempDir()
+
+    val importStatements =
+      """
+        |import java.util.Map;
+        |import org.apache.spark.api.plugin.*;

Review comment:
       What about using the qualified class name to avoid adding the new parameter `preClassDefinitionBlock`?

##########
File path: core/src/main/scala/org/apache/spark/TestUtils.scala
##########
@@ -179,11 +179,18 @@ private[spark] object TestUtils {
       destDir: File,
       toStringValue: String = "",
       baseClass: String = null,
-      classpathUrls: Seq[URL] = Seq.empty): File = {
+      classpathUrls: Seq[URL] = Seq.empty,
+      preClassDefinitionBlock: String = "",
+      implementsClasses: Seq[String] = Seq.empty,
+      extraCodeBody: String = ""): File = {
     val extendsText = Option(baseClass).map { c => s" extends ${c}" }.getOrElse("")
+    val implementsText = implementsClasses.map(", " + _).mkString

Review comment:
       maybe?
   ```suggestion
       val implementsText =
         s"implements ${implementsClasses ++ Seq("java.io.Serializable").mkString(", ")}"
   ```

##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,77 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    val tempDir = Utils.createTempDir()

Review comment:
       clean it up at the end of the test? (though I know it will be cleaned by shutdown hook anyway.)

##########
File path: core/src/main/scala/org/apache/spark/executor/Executor.scala
##########
@@ -227,6 +222,11 @@ private[spark] class Executor(
 
   metricsPoller.start()
 
+  // Plugins need to load using a class loader that includes the executor's user classpath

Review comment:
       Could you add comments to explain why we need to initialize plugin after heartbeater?




----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653971626


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972321


   **[Test build #124994 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124994/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657958137


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657877402


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656573871


   **[Test build #125591 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125591/testReport)** for PR 29002 at commit [`1ccb1b7`](https://github.com/apache/spark/commit/1ccb1b73f50f3eeb0bd5b05b7d59c6165e841e55).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653959474






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653969355


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972998


   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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654382124


   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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663802206






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656571024


   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] SparkQA removed a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658153415


   **[Test build #125831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125831/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-662473704


   Yeah I think this is ok, especially since normally if your plugin is less then heartbeat time there shouldn't be any difference. I would like to test this with a couple scenarios though.
   
   When we first put this in, I brought up that the plugin could block other executor functionality by running a long time, one thought was we could put plugin into another thread so it didn't block the executor. I think the thought at the time was if you have a long running executor plugin, the user should handle that by doing it in another thread or something.   Now you could make the argument you have something long running in the plugin and you want to block the executor for other initialization.  The heartbeat is 10seconds by default and I thought the timeout was the network timeout by default (120s), which seems like a really long time.  I guess if you change those timeouts, but even then seems like a long time.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654059042






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654059052


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124999/
   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] sarutak commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r460346580



##########
File path: core/src/main/scala/org/apache/spark/executor/Executor.scala
##########
@@ -227,6 +222,13 @@ private[spark] class Executor(
 
   metricsPoller.start()

Review comment:
       There is no strong reason so I'll put it just right after heartbeater.




----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659708361


   **[Test build #125997 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125997/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).
    * This patch **fails PySpark pip packaging 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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657877402






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653939712


   **[Test build #124964 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124964/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659020113






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659142838


   **[Test build #125932 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125932/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658862178


   **[Test build #125895 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125895/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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] sarutak edited a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656006944


   `ExecutorRegistered` is regarded as the first heartbeat message although it's not the real `Heartbeat`. 
   https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala#L114
   
   So If we reorder the initialization, it should be placed before the registration, not after heartbeater started.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653973003


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124994/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660464419






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659605710






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660481361


   **[Test build #126105 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126105/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).
    * 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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663835250






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658150610






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654382697


   **[Test build #125110 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125110/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657761388


   **[Test build #125789 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125789/testReport)** for PR 29002 at commit [`dba4c91`](https://github.com/apache/spark/commit/dba4c91fedba50ead3df311f440b4c9c4d970242).


----------------------------------------------------------------
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 pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655851569


   > As soon as we heartbeat to driver, we will start getting task assignment, etc - how does the interact with a plugin which has not yet completed initialization ?
   
   I wonder if this is correct.  Executor only gets task assignment after it sends `LaunchedExecutor` to driver:
   
   https://github.com/apache/spark/blob/3946b243284fbd3bd98b456115ae194ad49fe8fe/core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala#L156-L158
   
   
   After heartbeater started, the executor can send heartbeat to the driver asynchronically and the initialization of the plugin can keep going after heartbeater starting. It doesn't matter if it takes a long time because we'd send heartbeat now but no task assignment since `LaunchedExecutor` hasn't been sent.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663802095


   **[Test build #126520 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126520/testReport)** for PR 29002 at commit [`0b8da96`](https://github.com/apache/spark/commit/0b8da9620e736fc5cb485c349866593033b1fe33).
    * 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] SparkQA removed a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653970845


   **[Test build #124993 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124993/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655505147


   **[Test build #125316 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125316/testReport)** for PR 29002 at commit [`1ccb1b7`](https://github.com/apache/spark/commit/1ccb1b73f50f3eeb0bd5b05b7d59c6165e841e55).
    * 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] AmplabJenkins removed a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659709389


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125997/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655510803


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663790650






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663790544


   **[Test build #126520 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126520/testReport)** for PR 29002 at commit [`0b8da96`](https://github.com/apache/spark/commit/0b8da9620e736fc5cb485c349866593033b1fe33).


----------------------------------------------------------------
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 pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656154660


   > So If we reorder the initialization, it should be placed before the registration, not after heartbeater started right?
   
   Ideally, that way should work as well. However, it seems plugin initialization relies on some internal instances of `Executor`. So, if we want to initialize plugin out of the executor, I'm afraid we need more changes.
   
   BTW, I think your current implementation can already work as I mentioned above.  


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655667994


   @Ngone51 
   >Is this from a real issue?
   >
   >Shall we do the same thing for DriverPlugin or we already did?
   It happens not in the production but an environment for experiment.
   I confirmed we don't need to reorder the initialization for DriverPlugin because executors send hearbeat to the driver but the driver doesn't send it to self.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972563






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658860191






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653971613


   **[Test build #124993 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124993/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).
    * This patch **fails build dependency 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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653939727






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655510817


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125316/
   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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657912900


   According to the log for the previous unit test, the reason why the new test failed seems not to related to heartbeat.
   It seems to have taken about 30 secs until Executor started.
   
   ```
   20/07/13 13:49:15.501 redirect stderr for command /home/jenkins/workspace/SparkPullRequestBuilder/bin/spark-submit INFO Utils: SLF4J: Class path contains multiple SLF4J bindings.
   ```
   ```
   20/07/13 13:49:43.912 dispatcher-Executor INFO Executor: Starting executor ID 0 on host 192.168.10.25
   ```
   
   https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125789/artifact/core/target/unit-tests.log
   https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125789/artifact/work/app-20200713134929-0000/0/target/unit-tests.log
   
   So, I'll extend the timeout.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653980672






----------------------------------------------------------------
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] asfgit closed pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #29002:
URL: https://github.com/apache/spark/pull/29002


   


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656574275






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972989


   **[Test build #124994 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124994/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).
    * This patch **fails build dependency 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] mridulm commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656008648


   You are right @Ngone51, I misread the proposed 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] AmplabJenkins removed a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654383193






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656573871


   **[Test build #125591 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125591/testReport)** for PR 29002 at commit [`1ccb1b7`](https://github.com/apache/spark/commit/1ccb1b73f50f3eeb0bd5b05b7d59c6165e841e55).


----------------------------------------------------------------
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 pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665412590


   LGTM


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660464419






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663821652






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663790650






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659016278


   **[Test build #125895 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125895/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).
    * This patch **fails Spark unit tests**.
    * This patch **does not merge 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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655337806






----------------------------------------------------------------
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 a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r458814226



##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,73 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    withTempDir { tempDir =>
+      val sparkPluginCodeBody =
+        """
+          |@Override
+          |public org.apache.spark.api.plugin.ExecutorPlugin executorPlugin() {
+          |  return new TestExecutorPlugin();
+          |}
+          |
+          |@Override
+          |public org.apache.spark.api.plugin.DriverPlugin driverPlugin() { return null; }
+        """.stripMargin
+      val executorPluginBody =
+        """
+          |@Override
+          |public void init(
+          |    org.apache.spark.api.plugin.PluginContext ctx,
+          |    java.util.Map<String, String> extraConf) {
+          |  try {
+          |    Thread.sleep(30 * 1000);
+          |  } catch (InterruptedException e) {
+          |    throw new RuntimeException(e);
+          |  }
+          |}
+        """.stripMargin
+
+      val compiledExecutorPlugin = TestUtils.createCompiledClass(
+        "TestExecutorPlugin",
+        tempDir,
+        "",
+        null,
+        Seq.empty,
+        Seq("org.apache.spark.api.plugin.ExecutorPlugin"),
+        executorPluginBody)
+
+      val thisClassPath =
+        sys.props("java.class.path").split(File.pathSeparator).map(p => new File(p).toURI.toURL)
+      val compiledSparkPlugin = TestUtils.createCompiledClass(
+        "TestSparkPlugin",
+        tempDir,
+        "",
+        null,
+        Seq(tempDir.toURI.toURL) ++ thisClassPath,
+        Seq("org.apache.spark.api.plugin.SparkPlugin"),
+        sparkPluginCodeBody)
+
+      val jarUrl = TestUtils.createJar(
+        Seq(compiledSparkPlugin, compiledExecutorPlugin),
+        new File(tempDir, "testPlugin.jar"))
+
+      val unusedJar = TestUtils.createJarWithClasses(Seq.empty)
+      val args = Seq(
+        "--class", SimpleApplicationTest.getClass.getName.stripSuffix("$"),
+        "--name", "testApp",
+        "--master", "local-cluster[1,1,1024]",
+        "--conf", "spark.plugins=TestSparkPlugin",
+        "--conf", "spark.storage.blockManagerSlaveTimeoutMs=" + 10 * 1000,
+        "--conf", "spark.network.timeoutInterval=" + 10 * 1000,
+        "--conf", "spark.executor.heartbeatInterval=" + 5 * 1000,
+        "--conf", "spark.executor.extraClassPath=" + jarUrl.toString,
+        "--conf", "spark.driver.extraClassPath=" + jarUrl.toString,
+        "--conf", "spark.ui.enabled=false",
+        unusedJar.toString)
+      SparkSubmitSuite.runSparkSubmit(args, timeout = 2.minutes)

Review comment:
       how long does this test take?  30+ seconds, seems like a long test that is going to extend our unit test times.




----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658239160


   **[Test build #125831 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125831/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).
    * This patch **fails PySpark 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] AmplabJenkins commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656725013






----------------------------------------------------------------
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] HyukjinKwon commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-666494833


   Would you guys mind taking a look for the flaky tests added here? It seems failing in GitHub Actions as below:
   
   ```
   [info] - SPARK-32175: Plugin initialization should start after heartbeater started *** FAILED *** (32 seconds, 514 milliseconds)
   [info]   The code passed to failAfter did not complete within 30000000000 nanoseconds. (SparkSubmitSuite.scala:1438)
   [info]   org.scalatest.exceptions.TestFailedDueToTimeoutException:
   [info]   at java.lang.Thread.getStackTrace(Thread.java:1559)
   [info]   at org.scalatest.concurrent.TimeLimits.failAfterImpl(TimeLimits.scala:234)
   [info]   at org.scalatest.concurrent.TimeLimits.failAfterImpl$(TimeLimits.scala:233)
   [info]   at org.apache.spark.deploy.SparkSubmitSuite$.failAfterImpl(SparkSubmitSuite.scala:1419)
   [info]   at org.scalatest.concurrent.TimeLimits.failAfter(TimeLimits.scala:230)
   [info]   at org.scalatest.concurrent.TimeLimits.failAfter$(TimeLimits.scala:229)
   [info]   at org.apache.spark.deploy.SparkSubmitSuite$.failAfter(SparkSubmitSuite.scala:1419)
   [info]   at org.apache.spark.deploy.SparkSubmitSuite$.runSparkSubmit(SparkSubmitSuite.scala:1438)
   [info]   at org.apache.spark.executor.ExecutorSuite.$anonfun$new$17(ExecutorSuite.scala:469)
   [info]   at org.apache.spark.executor.ExecutorSuite.$anonfun$new$17$adapted(ExecutorSuite.scala:407)
   [info]   at org.apache.spark.SparkFunSuite.withTempDir(SparkFunSuite.scala:170)
   [info]   at org.apache.spark.executor.ExecutorSuite.$anonfun$new$16(ExecutorSuite.scala:407)
   [info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
   [info]   at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
   [info]   at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
   [info]   at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
   [info]   at org.scalatest.Transformer.apply(Transformer.scala:22)
   [info]   at org.scalatest.Transformer.apply(Transformer.scala:20)
   [info]   at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:189)
   [info]   at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:158)
   [info]   at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:187)
   ```
   
   https://github.com/apache/spark/runs/928145740
   https://github.com/apache/spark/runs/928147695
   https://github.com/apache/spark/runs/928059788
   https://github.com/apache/spark/runs/927624834
   https://github.com/apache/spark/runs/927483940
   https://github.com/apache/spark/runs/925705459
   https://github.com/apache/spark/runs/925169167
   https://github.com/apache/spark/runs/925406406
   https://github.com/apache/spark/runs/923441051
   
   The failure seems pretty frequent.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659204047






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653980498


   **[Test build #124999 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124999/testReport)** for PR 29002 at commit [`d724179`](https://github.com/apache/spark/commit/d72417972ef3d49891f6cc34c5468113ca3abdb7).


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972062


   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] sarutak commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r453895184



##########
File path: core/src/main/scala/org/apache/spark/TestUtils.scala
##########
@@ -179,11 +179,18 @@ private[spark] object TestUtils {
       destDir: File,
       toStringValue: String = "",
       baseClass: String = null,
-      classpathUrls: Seq[URL] = Seq.empty): File = {
+      classpathUrls: Seq[URL] = Seq.empty,
+      preClassDefinitionBlock: String = "",
+      implementsClasses: Seq[String] = Seq.empty,
+      extraCodeBody: String = ""): File = {
     val extendsText = Option(baseClass).map { c => s" extends ${c}" }.getOrElse("")
+    val implementsText = implementsClasses.map(", " + _).mkString

Review comment:
       Thanks. To be much simpler, I'll make it `(implementsClasses :+ "java.io.Serializable").mkString(", ")`.




----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659203147


   **[Test build #125932 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125932/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).
    * 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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653969554






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663835056


   **[Test build #126533 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126533/testReport)** for PR 29002 at commit [`0b8da96`](https://github.com/apache/spark/commit/0b8da9620e736fc5cb485c349866593033b1fe33).
    * 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] sarutak edited a comment on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655667994


   @Ngone51 
   >Is this from a real issue?
   >
   >Shall we do the same thing for DriverPlugin or we already did?
   
   It happens not in the production but an environment for experiment.
   I confirmed we don't need to reorder the initialization for DriverPlugin because executors send hearbeat to the driver but the driver doesn't send it to self.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-665373210






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657916150


   **[Test build #125798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125798/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653971626






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972998






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659604878


   **[Test build #125997 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125997/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
tgravescs commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663572512


   ok looked at this a bit more and tested a few scenarios, I think this is fine, I would like it put right after heartbeater unless we have reason and would be nice to get test to run faster.


----------------------------------------------------------------
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] HyukjinKwon commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-666845566


   Sure, removing sounds fine and simpler to me.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658240283


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125831/
   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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-666846968


   O.K, I'll make a PR for removing it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655510803






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659605710






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653959476


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124964/
   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654059042


   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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657957833


   **[Test build #125798 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125798/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).
    * This patch **fails PySpark pip packaging 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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653980672






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-658240267


   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] Ngone51 commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r459373146



##########
File path: core/src/test/scala/org/apache/spark/executor/ExecutorSuite.scala
##########
@@ -402,6 +403,73 @@ class ExecutorSuite extends SparkFunSuite
     assert(taskMetrics.getMetricValue("JVMHeapMemory") > 0)
   }
 
+  test("SPARK-32175: Plugin initialization should start after heartbeater started") {
+    withTempDir { tempDir =>
+      val sparkPluginCodeBody =
+        """
+          |@Override
+          |public org.apache.spark.api.plugin.ExecutorPlugin executorPlugin() {
+          |  return new TestExecutorPlugin();
+          |}
+          |
+          |@Override
+          |public org.apache.spark.api.plugin.DriverPlugin driverPlugin() { return null; }
+        """.stripMargin
+      val executorPluginBody =
+        """
+          |@Override
+          |public void init(
+          |    org.apache.spark.api.plugin.PluginContext ctx,
+          |    java.util.Map<String, String> extraConf) {
+          |  try {
+          |    Thread.sleep(30 * 1000);
+          |  } catch (InterruptedException e) {
+          |    throw new RuntimeException(e);
+          |  }
+          |}
+        """.stripMargin
+
+      val compiledExecutorPlugin = TestUtils.createCompiledClass(
+        "TestExecutorPlugin",
+        tempDir,
+        "",
+        null,
+        Seq.empty,
+        Seq("org.apache.spark.api.plugin.ExecutorPlugin"),
+        executorPluginBody)
+
+      val thisClassPath =
+        sys.props("java.class.path").split(File.pathSeparator).map(p => new File(p).toURI.toURL)
+      val compiledSparkPlugin = TestUtils.createCompiledClass(
+        "TestSparkPlugin",
+        tempDir,
+        "",
+        null,
+        Seq(tempDir.toURI.toURL) ++ thisClassPath,
+        Seq("org.apache.spark.api.plugin.SparkPlugin"),
+        sparkPluginCodeBody)
+
+      val jarUrl = TestUtils.createJar(
+        Seq(compiledSparkPlugin, compiledExecutorPlugin),
+        new File(tempDir, "testPlugin.jar"))
+
+      val unusedJar = TestUtils.createJarWithClasses(Seq.empty)
+      val args = Seq(
+        "--class", SimpleApplicationTest.getClass.getName.stripSuffix("$"),
+        "--name", "testApp",
+        "--master", "local-cluster[1,1,1024]",
+        "--conf", "spark.plugins=TestSparkPlugin",
+        "--conf", "spark.storage.blockManagerSlaveTimeoutMs=" + 10 * 1000,
+        "--conf", "spark.network.timeoutInterval=" + 10 * 1000,
+        "--conf", "spark.executor.heartbeatInterval=" + 5 * 1000,
+        "--conf", "spark.executor.extraClassPath=" + jarUrl.toString,
+        "--conf", "spark.driver.extraClassPath=" + jarUrl.toString,
+        "--conf", "spark.ui.enabled=false",
+        unusedJar.toString)
+      SparkSubmitSuite.runSparkSubmit(args, timeout = 2.minutes)

Review comment:
       It should be enough if the sleep time of the plugin is a little bit bigger than `spark.executor.heartbeatInterval`? e.g. 8 or 10 seconds.




----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659604160


   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] mridulm commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
mridulm commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-655659770


   As soon as we heartbeat to driver, we will start getting task assignment, etc - how does the interact with a plugin which has not yet completed initialization ?
   Instead of changing order, why not document that inordinate delays in plugin initialization should be avoided and deferred to seperate thread/managed by plugin.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-660464379


   **[Test build #126105 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126105/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-666733392


   @HyukjinKwon I'll look into that. Should we ignore the test first?


----------------------------------------------------------------
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] sarutak commented on a change in pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29002:
URL: https://github.com/apache/spark/pull/29002#discussion_r453895184



##########
File path: core/src/main/scala/org/apache/spark/TestUtils.scala
##########
@@ -179,11 +179,18 @@ private[spark] object TestUtils {
       destDir: File,
       toStringValue: String = "",
       baseClass: String = null,
-      classpathUrls: Seq[URL] = Seq.empty): File = {
+      classpathUrls: Seq[URL] = Seq.empty,
+      preClassDefinitionBlock: String = "",
+      implementsClasses: Seq[String] = Seq.empty,
+      extraCodeBody: String = ""): File = {
     val extendsText = Option(baseClass).map { c => s" extends ${c}" }.getOrElse("")
+    val implementsText = implementsClasses.map(", " + _).mkString

Review comment:
       Thanks. To be more simpler, I'll make it `(implementsClasses :+ "java.io.Serializable").mkString(", ")`.




----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-666751921


   Yeah, I wonder we need to increase the times. The testcase is to detect regressions which can happen in the future but I've left the comment about the order so removing the test can be fine.
   @HyukjinKwon , what do you think?


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654502684


   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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654383193






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-663802206


   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 commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-653972563






----------------------------------------------------------------
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] sarutak commented on pull request #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-656247590


   > Ideally, that way should work as well. However, it seems plugin initialization relies on some internal instances of Executor. So, if > we want to initialize plugin out of the executor, I'm afraid we need more changes.
   
   Yes, if we reorder the initialization with another way, the initialization would be put on executor backends like `CoarseGrainedExecutorBackend` and we might need more changes as you are afraid.
   
   > BTW, I think your current implementation can already work as I mentioned above.
   Yeah, I misunderstood. `LaunchExecutor` is sent after `Executor` is instansiated so the current implementation works.


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659709389






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-654502684






----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-659604878


   **[Test build #125997 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125997/testReport)** for PR 29002 at commit [`d768385`](https://github.com/apache/spark/commit/d768385caac9c79c456de87a4afd72298dda46db).


----------------------------------------------------------------
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 #29002: [SPARK-32175][CORE] Fix the order between initialization for ExecutorPlugin and starting heartbeat thread

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29002:
URL: https://github.com/apache/spark/pull/29002#issuecomment-657876826


   **[Test build #125789 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125789/testReport)** for PR 29002 at commit [`dba4c91`](https://github.com/apache/spark/commit/dba4c91fedba50ead3df311f440b4c9c4d970242).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `         |public class $className $extendsText $implementsText `


----------------------------------------------------------------
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