You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by HyukjinKwon <gi...@git.apache.org> on 2017/06/16 02:46:00 UTC
[GitHub] spark pull request #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in ga...
GitHub user HyukjinKwon opened a pull request:
https://github.com/apache/spark/pull/18320
[SPARK-21093][R] Avoid mcfork in R's daemon in gapply/gapplyCollect tests
## What changes were proposed in this pull request?
`mcfork` in R looks opening a pipe ahead but the existing logic does not properly close it when it is executed hot. This leads to the failure of more forking due to the limit for number of files open.
This hot execution looks particularly for `gapply`/`gapplyCollect`. For unknown reason, this happens more easily in CentOS and could be reproduced in Mac too.
All the details are described in https://issues.apache.org/jira/browse/SPARK-21093
This PR proposes simply to avoid reusing that daemon but each process from JVM that look terminating all correctly.
## How was this patch tested?
I ran the codes below on both CentOS and Mac.
```r
df <- createDataFrame(list(list(1L, 1, "1", 0.1)), c("a", "b", "c", "d"))
collect(gapply(df, "a", function(key, x) { x }, schema(df)))
collect(gapply(df, "a", function(key, x) { x }, schema(df)))
... # 30 times
```
Also, now it passes R tests on CentOS as below:
```
SparkSQL functions: Spark package found in SPARK_HOME: .../spark
..............................................................................................................................................................
..............................................................................................................................................................
..............................................................................................................................................................
..............................................................................................................................................................
..............................................................................................................................................................
....................................................................................................................................
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HyukjinKwon/spark SPARK-21093
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18320.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18320
----
commit 505d75f0e9a90481f96d0f1fefd4f9baaa38ee7d
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-16T02:37:53Z
Avoid mcfork in R's daemon in gapply tests
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78282/testReport)** for PR 18320 at commit [`04bb37a`](https://github.com/apache/spark/commit/04bb37a6d8d4387365f6d46cb8e2c6fbe912351d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I checked R tests and the reproducer in the PR description passed fine on CentOS.
> The other thing that would be good to manually test (since its not easy to write a unit test) is to see if the selectTimeout becomes NULL when we expect it to.
Yes, I checked by manually printing out and checked the logs. I just double checked it became `NULL` when we want.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78278 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78278/testReport)** for PR 18320 at commit [`a61141b`](https://github.com/apache/spark/commit/a61141b3d7a65a682a4c9f54068235af3c5abf54).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78255 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78255/testReport)** for PR 18320 at commit [`fa7a226`](https://github.com/apache/spark/commit/fa7a2261e915ae11f3c83139bcf8d9f62a5929c6).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122605437
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
This can be tested as below:
```bash
vi tmp.R
```
copy and paste
```R
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
parallel:::mcexit(0L)
}
print(parallel:::selectChildren())
Sys.sleep(4L)
print(parallel:::selectChildren())
```
This prints
```bash
[1] TRUE
[1] 2505
```
For return values, please refer https://stat.ethz.ch/R-manual/R-devel/library/parallel/html/children.html.
For testing resource leak, this can be tested as below:
```R
for(i in 0:100000) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
parallel:::mcexit(0L)
}
exitChildren <- parallel:::selectChildren()
if (is.integer(exitChildren)) {
lapply(exitChildren, function(x) { tools::pskill(x, tools::SIGUSR1) })
}
}
```
The original code below:
```R
for(i in 0:100000) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
tools::pskill(Sys.getpid(), tools::SIGUSR1)
parallel:::mcexit(0L)
}
}
```
will probably complain in `mcfork` due to the lack of resources (e.g., ran out of file descriptors).
Actual number can be checked via `watch -n 0.01 "lsof -c R | wc -l"` in another terminal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/18320
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I will check if this works on CentOS on Monday. What do you think about the current approach @felixcheung?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123717458
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
I ran several tests for this https://github.com/apache/spark/pull/18320#discussion_r122860310 and https://github.com/apache/spark/pull/18320#discussion_r123675088
```
vi tmp.R
```
copy and paste
```R
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
parallel:::mcexit(0L)
}
print("TRUE - the timeout was reached")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
```
and ...
```
Rscript tmp.R
```
If data is not available, this should wait as much as `timeout` up to my knowledge rather than returning `NULL`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123714768
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
One path described above is:
> 1. Every second if any socket connection is not available and if there are child workers
running.
So after forking succeeded and we want to do check per second, but after one second and if no children sent data, we turn back immediately to wait indefinitely. Doesn't it mean we don't actually do the logic every second but just for the first second?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123686706
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,54 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is -1 that indecides an exited child.
--- End diff --
`-1` to exit code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123675152
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,51 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
--- End diff --
Also, I referred the code bit in https://github.com/mllg/batchtools/blob/master/R/clusterFunctionsMulticore.R#L8-L18.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Yes, I believe you are correct and the daemon is already running but it avoids to use the problematic daemon - https://github.com/apache/spark/blob/478fbc866fbfdb4439788583281863ecea14e8af/core/src/main/scala/org/apache/spark/api/r/RRunner.scala#L363-L392 up to my knowledge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123416154
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
--- End diff --
hmm, this is pre-existing behavior, but generally I think waiting indefinitely for anything would be rather dangerous. probably would be good to follow up separately
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78280 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78280/testReport)** for PR 18320 at commit [`4954008`](https://github.com/apache/spark/commit/4954008884ff02a9eae9ea50586e86e8923fc593).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123686953
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,54 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
--- End diff --
nit: I think the comment on L59 and L63 can be updated to match more closely to the new behavior - that we are checking proper exit code is sent
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78517 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78517/testReport)** for PR 18320 at commit [`53e00d7`](https://github.com/apache/spark/commit/53e00d70871355e11b19f99c71fa34a92008ab27).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Let me leave a link https://github.com/apache/spark/pull/18456 for the related discussion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78519/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
retest this please. I don't think this PR caused that -9 issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78204 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78204/testReport)** for PR 18320 at commit [`1658a44`](https://github.com/apache/spark/commit/1658a44a18df93d58fdb5d0bd2cd19df841f5883).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
merged to master, thanks!!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
@felixcheung, @shivaram, I addressed the comments and ran some tests. I am positive of other tests I will proceed above. Before testing all, could we take a look to double check?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Hmmmm. I catefully suspect this one, in particular, the last non-comment change causes unknown test failure with -10 error code. Is it generally acceptable to revert a PR based on a wild guess? I have been investigating it but it is hard to reproduce.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78284 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78284/testReport)** for PR 18320 at commit [`d6f0ff2`](https://github.com/apache/spark/commit/d6f0ff275abd3b7641210427eea955c9f0ea8d86).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78203/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I was thinking we should not work around for now if this can't be fixed within `daemon.R` and it is not practically problematic which we don't know yet.
Let try to deal with a look for `daemon.R`. Probably, the best way is to fix this problem together. I will be back after thinking more and taking a look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78204 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78204/testReport)** for PR 18320 at commit [`1658a44`](https://github.com/apache/spark/commit/1658a44a18df93d58fdb5d0bd2cd19df841f5883).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
cc @shivaram too, could you take a look here too? I am very careful of this change and want to make sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
GitHub user HyukjinKwon reopened a pull request:
https://github.com/apache/spark/pull/18320
[SPARK-21093][R] Terminate R's worker processes in the parent of R's daemon to prevent a leak
## What changes were proposed in this pull request?
`mcfork` in R looks opening a pipe ahead but the existing logic does not properly close it when it is executed hot. This leads to the failure of more forking due to the limit for number of files open.
This hot execution looks particularly for `gapply`/`gapplyCollect`. For unknown reason, this happens more easily in CentOS and could be reproduced in Mac too.
All the details are described in https://issues.apache.org/jira/browse/SPARK-21093
This PR proposes simply to terminate R's worker processes in the parent of R's daemon to prevent a leak.
## How was this patch tested?
I ran the codes below on both CentOS and Mac with that configuration disabled/enabled.
```r
df <- createDataFrame(list(list(1L, 1, "1", 0.1)), c("a", "b", "c", "d"))
collect(gapply(df, "a", function(key, x) { x }, schema(df)))
collect(gapply(df, "a", function(key, x) { x }, schema(df)))
... # 30 times
```
Also, now it passes R tests on CentOS as below:
```
SparkSQL functions: Spark package found in SPARK_HOME: .../spark
..............................................................................................................................................................
..............................................................................................................................................................
..............................................................................................................................................................
..............................................................................................................................................................
..............................................................................................................................................................
....................................................................................................................................
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HyukjinKwon/spark SPARK-21093
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18320.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18320
----
commit 6e57ed2931afc5aec8c4b4bef72c157abcb68c46
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-16T02:37:53Z
Terminates forked processed in the parent process
commit 4eadafe3f009b1c70956c08c99302c1da34db6d4
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-17T09:21:37Z
Fix typo (renaming missed)
commit 18b3ee9a66df40658074511558f0cd36fc102df7
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-17T10:54:57Z
Rename x to c in lapply
commit 72ab1f2e8cafa1d5249a09279825444e3ca38b39
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-19T12:21:08Z
Update comments to describe the behaviour change
commit 6cba54c243123d25f479363d3dfd7eb92bb25599
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-20T01:02:26Z
Do not check every second if there is no worker running
commit 4954008884ff02a9eae9ea50586e86e8923fc593
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-20T09:04:03Z
Address comment
commit f3f57e46868e66b8f50268910c1eff494638059d
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-20T09:30:56Z
Fix comments
commit 04bb37a6d8d4387365f6d46cb8e2c6fbe912351d
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-20T09:32:29Z
Fix comments
commit d6f0ff275abd3b7641210427eea955c9f0ea8d86
Author: hyukjinkwon <gu...@gmail.com>
Date: 2017-06-20T09:39:21Z
Add more comments
commit 8b48274dc565dc5c6722e983c55494b0067bda72
Author: Hyukjin Kwon <gu...@gmail.com>
Date: 2017-06-20T10:00:05Z
Fix a typo
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
@felixcheung, BTW, is it okay as a PR alone as is?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122606765
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -47,8 +55,6 @@ while (TRUE) {
close(inputCon)
Sys.setenv(SPARKR_WORKER_PORT = port)
try(source(script))
- # Set SIGUSR1 so that child can exit
- tools::pskill(Sys.getpid(), tools::SIGUSR1)
parallel:::mcexit(0L)
--- End diff --
BTW, up to my knowledge, this should work alone, at least in C (< I didn't test). Even if it was a bug in R and is fixed in the future, I guess the current logics would still work with the current status.
```R
arbitrary <- 9999
tools::pskill(arbitrary, tools::SIGUSR1)
```
https://stat.ethz.ch/R-manual/R-devel/library/tools/html/pskill.html
> it silently ignores invalid values of its arguments, including zero or negative pids.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78280/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
(I tested the current status with Mac OS for now).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the issue:
https://github.com/apache/spark/pull/18320
LGTM. The update looks good. Thanks for the thorough testing. We could also post a note on the dev list about this change and especially ask people who use `dapply` or `gapply` or the old `RDD` based API to test this out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Yes, I believe you are correct but the daemon is already running but it avoids to use the problematic daemon - https://github.com/apache/spark/blob/478fbc866fbfdb4439788583281863ecea14e8af/core/src/main/scala/org/apache/spark/api/r/RRunner.scala#L363-L392 up to my knowledge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123715868
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
It becomes `NULL` if there are no children. It returns `TRUE` if there are children here. So, we always turn back to wait indefinitely only if there are no children at all.
https://stat.ethz.ch/R-manual/R-devel/library/parallel/html/children.html
> TRUE is the timeout was reached, FALSE if an error occurred (e.g., if the master process was interrupted) or an integer vector of process IDs with children that have data available, or NULL if there are no children.
Would this answer your question (or did I maybe misunderstand) ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123564501
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
--- End diff --
Its less of waiting indefinitely but its waiting for input from the JVM socket -- I think this is the right behavior by design
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78499/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I will re-check these after addressing the comments:
Mac OS
- [ ] jobs with many stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] jobs with long stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] NULL when we expect it - https://github.com/apache/spark/pull/18320#issuecomment-309940653
- [ ] sleep before connection - https://github.com/apache/spark/pull/18320#discussion_r122687051
- [ ] full tests
- [ ] dapply/gapply described in the PR
CentOS
- [ ] jobs with many stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] jobs with long stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] NULL when we expect it - https://github.com/apache/spark/pull/18320#issuecomment-309940653
- [ ] sleep before connection - https://github.com/apache/spark/pull/18320#discussion_r122687051
- [ ] full tests
- [ ] dapply/gapply described in the PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
thx - I think more importantly, does the issue manifest when someone manually call gapply in a similar way on RHEL/CentOS? We could workaround the test failure, but if user can use into this in normal use then we need to address this within gapply
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Thanks for a close look. I am pretty confident of the current state now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123440068
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
--- End diff --
I think [this line](https://github.com/s-u/multicore/blob/e9d9bf21e6cf08e24cfe54d762379b4fa923765b/src/fork.c#L440) checks if any data from children is available via file descriptors (pipes I believe).
The child do not send any data back to the parent (via `parallel:::sendMaster` API and etc.). So, `FD_ISSET` will only be true on the close of the pipe (as this is a event to the pipe), which I guess happens when `exit()` is called properly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123419616
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
--- End diff --
right, I see your reference here https://github.com/apache/spark/pull/18320#discussion_r122639738
but I'm not 100% getting it when looking at the source code https://github.com/s-u/multicore/blob/e9d9bf21e6cf08e24cfe54d762379b4fa923765b/src/fork.c#L361
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78501 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78501/testReport)** for PR 18320 at commit [`9446c4a`](https://github.com/apache/spark/commit/9446c4a0e813b3e85673cb5db2c42fb003a28c59).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78279 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78279/testReport)** for PR 18320 at commit [`4d9469b`](https://github.com/apache/spark/commit/4d9469b419570dd91cc73c944b3c8460b94a4021).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
For normal usecases, I carefully suspect it might be fine because I executed 200 * ~10 tasks in a single machine quickly but I don't know if it happens frequently when it runs slowly in a cluster in a distributed manner.
At least, this was not reproduced when the number of fork executions is not many. Practically, it might be fine but need more investigation if this is important to prioritize this issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123719067
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
nit: `If it is NULL, there are no such children.` -> `If it is NULL, there are no any children.`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123700234
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,51 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
--- End diff --
With the change above, it printed:
```
[1] "Wait for 4 seconds to test the last child ..."
[1] "child PID: 86866 and parent will kill given: -1"
[1] "child PID: 86865 and parent will kill given: -1"
[1] "child PID: 86864 and sent a data: arbitrary"
[1] "child PID: 86863 and sent a PID: 86863"
[1] "Wait for 7 seconds more to test the last child ..."
[1] "child PID: 86864 and sent a data: 123"
```
It looks correct.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78147/testReport)** for PR 18320 at commit [`505d75f`](https://github.com/apache/spark/commit/505d75f0e9a90481f96d0f1fefd4f9baaa38ee7d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I will check if this works on CentOS on Monday. What do you think about the current approach @felixcheung?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
It looks we should exit children in the parent. Previously, the child looks sending the kill signal for itself. Killing those in the parent looks resolving this problem properly.
Before:
```r
for(i in 0:200) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
tools::pskill(Sys.getpid(), tools::SIGUSR1)
parallel:::mcexit(0L)
}
}
```
Roughly, after:
```r
for(i in 0:200) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
parallel:::mcexit(0L)
}
exitChildren <- parallel:::selectChildren()
if (is.integer(exitChildren)) {
lapply(exitChildren, function(x) { tools::pskill(x, tools::SIGUSR1) })
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78286 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78286/testReport)** for PR 18320 at commit [`8b48274`](https://github.com/apache/spark/commit/8b48274dc565dc5c6722e983c55494b0067bda72).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122609177
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
Ah, yes. It looks confusing. Probably, let me add more comments around here. I intended to do ...
A: When data is arrived in `daemon.R`:
- `ready` is `TRUE` and terminates children, `worker.R`, if they finished their works.
- Launch other `worker.R` children.
B: When data is not arrived in `daemon.R`:
- `ready` is `FALSE` and terminates children, `worker.R`, if they finished their works, every second.
If we do this within `if (ready) {`, the child processes will remain until A case happens.
Forked children do some jobs and if the parent checks if any is finished right after forking by `parallel:::selectChildren()` will probably not return the finished children PIDs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78148 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78148/testReport)** for PR 18320 at commit [`52c8abf`](https://github.com/apache/spark/commit/52c8abf9551e126f75ef0aa0a042f1ebd13e8d47).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
that's very interesting. that code has been around for 2 years - to be honest I'm not 100% sure about what it is doing.
perhaps this could also be fixed with a lower number of partitions?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122564686
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
--- End diff --
So, I intended to terminates the children for every second _or_ before launching other workers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Sure, I also think we should be very cateful and should not target 2.2.0. I will take a look for comments for sure. Please let me know if there is any concern more. I am willing to double check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122860937
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,42 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+selectTimeout <- function() {
+ if (is.null(parallel:::selectChildren(timeout = 0))) {
--- End diff --
And ... just in case, these are can be checked via both commands as below:
```
watch -n 0.01 "ps -fe | grep /exec/R"
```
checks forked processes.
```
watch -n 0.01 "lsof -c R | wc -l"
```
check open files (and file descriptors & pipes).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78280 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78280/testReport)** for PR 18320 at commit [`4954008`](https://github.com/apache/spark/commit/4954008884ff02a9eae9ea50586e86e8923fc593).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78203 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78203/testReport)** for PR 18320 at commit [`74deb23`](https://github.com/apache/spark/commit/74deb2341ba2aabd58f280b5f7e463b6cad78c41).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122847863
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
--- End diff --
To make my point a bit more clear, if there are no workers running, we can block on this `select` and dont need the one second timeout in that case ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122606530
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -47,8 +55,6 @@ while (TRUE) {
close(inputCon)
Sys.setenv(SPARKR_WORKER_PORT = port)
try(source(script))
- # Set SIGUSR1 so that child can exit
- tools::pskill(Sys.getpid(), tools::SIGUSR1)
parallel:::mcexit(0L)
--- End diff --
`mcexit` alone does not terminate the child.
With terminal A:
```R
R
```
With terminal B:
```bash
watch -n 0.01 "ps -fe | grep /bin/exec/R"
```
with terminal A:
```R
for(i in 0:100) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
parallel:::mcexit(0L)
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122639738
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
Definitely for comments.
Maybe I missed your point. Children will only return their PID on exit and `parallel:::selectChildren()` will only return children PIDs that finished they work and called `parallel:::mcexit(0L)` (related test was done https://github.com/apache/spark/pull/18320#discussion_r122605437) up to my knowledge. So, even if connecting to JVM is delayed in `worker.R` and `RRunner.scala`, it won't matter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78508/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122803981
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
--- End diff --
@HyukjinKwon Do we know what the old behavior was doing ? I'm thinking if we would spend more time in the loop below now and be less responsive to the `inputCon`. One thing -- Can we control the timeout based on whether there are any children running or not ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123718675
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
Oh, cool. LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123681647
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
--- End diff --
right, but in the odd case that JVM died before making the socket connection, this R process will wait forever?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
**jobs with many stages**:
I tested the codes below:
```R
```r
df <- createDataFrame(list(list(1L, 1, "1", 0.1)), c("a", "b", "c", "d"))
for(i in 0:90) {
df <- (gapply(df, "a", function(key, x) { x }, schema(df)))
}
collect(df)
```
More iteration produced `StackOverflowError` in my local and CentOS. This created 18201 tasks with 92 stages.
**jobs with long stages**:
I made the change as below:
```r
df <- createDataFrame(list(list(1L, 1, "1", 0.1)), c("a", "b", "c", "d"))
collect(dapply(repartition(df, 8), function(x) { x }, schema(df)))
```
after manual changes as below:
```r
outputCon <- socketConnection(
port = port, blocking = TRUE, open = "wb", timeout = connectionTimeout)
+Sys.sleep(600L)
+
# read the index of the current partition inside the RDD
partition <- SparkR:::readInt(inputCon)
```
This took 10 mins (default cores in executors was 8).
It looks both were fine. Would this address your concern enough @felixcheung?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78282/testReport)** for PR 18320 at commit [`04bb37a`](https://github.com/apache/spark/commit/04bb37a6d8d4387365f6d46cb8e2c6fbe912351d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123675088
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,51 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
--- End diff --
I tested this logics as below:
```
vi tmp.R
```
copied and pasted
```r
kill <- function(children) {
lapply(children, function(child) {
data <- parallel:::readChild(child)
if (is.raw(data)) {
if (unserialize(data) == -1) {
print(paste("child PID:", child, "and parent will kill given:", unserialize(data)))
tools::pskill(child, tools::SIGUSR1)
} else {
print(paste("child PID:", child, "and sent a data:", unserialize(data)))
}
} else {
print(paste("child PID:", child, "and sent a PID:", data))
}
})
}
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
# Send no data
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
# Send custom data
parallel:::sendMaster("arbitrary")
parallel:::sendMaster(123)
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
# Send explicit singal.
parallel:::mcexit(0L, send = -1)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(10L)
# Check if `readChild` waits
parallel:::mcexit(0L, send = -1)
}
}
}
}
print("Wait for 4 seconds to test the last child ...")
Sys.sleep(4L)
invisible(kill(parallel:::selectChildren(timeout = 0)))
print("Wait for 7 seconds more to test the last child ...")
Sys.sleep(7L)
invisible(kill(parallel:::selectChildren(timeout = 0)))
```
```
[1] "Wait for 4 seconds to test the last child ..."
[1] "child PID: 7405 and parent will kill given: -1"
[1] "child PID: 7404 and sent a data: arbitrary"
[1] "child PID: 7403 and sent a PID: 7403"
[1] "Wait for 7 seconds more to test the last child ..."
[1] "child PID: 7406 and parent will kill given: -1"
[1] "child PID: 7404 and sent a data: 123"
```
and ran
```
Rscript tmp.R
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78278 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78278/testReport)** for PR 18320 at commit [`a61141b`](https://github.com/apache/spark/commit/a61141b3d7a65a682a4c9f54068235af3c5abf54).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/18320
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122860310
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,42 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+selectTimeout <- function() {
+ if (is.null(parallel:::selectChildren(timeout = 0))) {
--- End diff --
This can be tested as below:
```r
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
parallel:::mcexit(0L)
}
print("TRUE - the timeout was reached")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
Sys.sleep(4L)
print("PID - child is exited (but not terminated)")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
invisible(lapply(parallel:::selectChildren(timeout = 0), function(x) { tools::pskill(x, tools::SIGUSR1) }))
Sys.sleep(1L)
print("NULL - if there are no children")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
```
Per the documentation - https://stat.ethz.ch/R-manual/R-devel/library/parallel/html/children.html
> selectChildren returns `TRUE` is the timeout was reached, `FALSE` if an error occurred (e.g., if the master process was interrupted) or an integer vector of process IDs with children that have data available, or` NULL` if there are no children.
I also manually built this and checked if it really wait indefinitely with printing out.
via running
```r
df <- createDataFrame(list(list(1L, 1, "1", 0.1)), c("a", "b", "c", "d"))
collect(gapply(df, "a", function(key, x) { x }, schema(df)))
collect(gapply(df, "a", function(key, x) { x }, schema(df)))
... # 30 times
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123688218
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,51 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
--- End diff --
Definitely.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78284/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78519 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78519/testReport)** for PR 18320 at commit [`7d61153`](https://github.com/apache/spark/commit/7d611537244ef1d5226e8983e0818c2d2e65ce55).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78519 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78519/testReport)** for PR 18320 at commit [`7d61153`](https://github.com/apache/spark/commit/7d611537244ef1d5226e8983e0818c2d2e65ce55).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78518/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78508 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78508/testReport)** for PR 18320 at commit [`53e00d7`](https://github.com/apache/spark/commit/53e00d70871355e11b19f99c71fa34a92008ab27).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I checked R tests and the reproducer in the PR description passed fine.
> The other thing that would be good to manually test (since its not easy to write a unit test) is to see if the selectTimeout becomes NULL when we expect it to.
Yes, I checked by manually printing out and checked the logs. I just double checked it became `NULL` when we want.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78281/testReport)** for PR 18320 at commit [`f3f57e4`](https://github.com/apache/spark/commit/f3f57e46868e66b8f50268910c1eff494638059d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122609234
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
The related test was performed here - https://github.com/apache/spark/pull/18320#discussion_r122605437
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78147/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122875289
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,42 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+selectTimeout <- function() {
+ if (is.null(parallel:::selectChildren(timeout = 0))) {
+ # Wait a socket connection indefinitely if there are no workers running.
+ NULL
--- End diff --
Nice - this is in line with what I was thinking. One thing: Can we maintain some state in our own code to avoid calling `select` twice ? What I mean is if `finishedChildren` was `NULL` (based on [1]) then we set a flag saying `timeout <- NULL` -- next time we fork some children we set this back to `1`. Will that work or am I missing something ?
[1] https://stat.ethz.ch/R-manual/R-devel/library/parallel/html/children.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78255 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78255/testReport)** for PR 18320 at commit [`fa7a226`](https://github.com/apache/spark/commit/fa7a2261e915ae11f3c83139bcf8d9f62a5929c6).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78513/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78501/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78447/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I suspect this is an issue in R. I will raise this issue in R community soon and share it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122695789
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,30 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it sends a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent sends the signal to children every second or right before
+ # launching other worker children from the following new socket connection.
+ #
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ finishedChildren <- parallel:::selectChildren(timeout = 0)
--- End diff --
This is 0 by default but I added to prevent conversion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78255/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
right, thanks! none of my comment would suggest code changes - they should all be comment changes, feel free to not restart all tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78523 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78523/testReport)** for PR 18320 at commit [`c5fe347`](https://github.com/apache/spark/commit/c5fe3478298d992a7e91515651179d10563af445).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122879650
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,42 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+selectTimeout <- function() {
+ if (is.null(parallel:::selectChildren(timeout = 0))) {
+ # Wait a socket connection indefinitely if there are no workers running.
+ NULL
--- End diff --
Sure, let me check this out soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122564740
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
`timeout` for `selectChildren` looks 0 by default. So, this does not look hanging here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78281/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
cc @felixcheung, @shivaram and @MLnick.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
also, perhaps this is a bug in RHEL/CentOS? it seems like mcfork only calls the OS fork function
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78147 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78147/testReport)** for PR 18320 at commit [`505d75f`](https://github.com/apache/spark/commit/505d75f0e9a90481f96d0f1fefd4f9baaa38ee7d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78269 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78269/testReport)** for PR 18320 at commit [`05b91b8`](https://github.com/apache/spark/commit/05b91b8fd30099a8a10ccbd89a552a917d095f62).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123717120
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
I have this question because you have a comment:
Only the process IDs of children sent data to the parent are returned below.
So I am not sure if there are no children with data sent to the parent, the returned value is NULL or other?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78269/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122608540
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
why not do this under `if (ready) {` similar to before?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78523/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78501 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78501/testReport)** for PR 18320 at commit [`9446c4a`](https://github.com/apache/spark/commit/9446c4a0e813b3e85673cb5db2c42fb003a28c59).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Yes, I guess it will pass if we reduce `spark.sql.shuffle.partitions` (< I didn't look carefully and test this either). Just to make sure (and to share what I investigated ...), from my code read,
With `spark.sparkr.use.daemon` enabled, for each task execution,
1. JVM ----start (if not started)--------> R daemon
2. JVM ----send port ------------------> R daemon ----fork with the port---> R worker
This looks being tested on OSs except for Windows.
With `spark.sparkr.use.daemon` disabled, for each task execution,
1. JVM ----forking processes from Java (expensive)-------------------------> R worker
This looks being tested only on Windows.
This PR proposes to switch this one to latter case (which was the former before) by avoiding calling the (already running from other execution) R daemon.
I am fine with giving a shot with reducing the number of partitions if you are fond of it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122804718
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,30 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it sends a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent sends the signal to children every second or right before
+ # launching other worker children from the following new socket connection.
+ #
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ finishedChildren <- parallel:::selectChildren(timeout = 0)
--- End diff --
Does the timeout of `0` not wait at all or does it block until it gets all the results ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78204/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78269 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78269/testReport)** for PR 18320 at commit [`05b91b8`](https://github.com/apache/spark/commit/05b91b8fd30099a8a10ccbd89a552a917d095f62).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78208 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78208/testReport)** for PR 18320 at commit [`99245fb`](https://github.com/apache/spark/commit/99245fb655cde49831737cbe5da78eaea55a51dc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
> Also I'd suggest not committing this to branch-2.2 -- if we want to just fix the CentOS tests we can have a different change for the older branches
agreed, this won't run as a part of CRAN checks so I think it's optional at this point to fix CentOS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123569536
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
--- End diff --
Hmm @felixcheung this is an good point - The thing is I think what @HyukjinKwon says might be valid given the current code path -- i.e. the child processes do not write anything back to the master process until exit. However that seems pretty fragile going forward ? What if we explicitly used `sendMaster` to send back an exited signal to the master ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123417186
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -47,9 +79,11 @@ while (TRUE) {
close(inputCon)
--- End diff --
please add comment here to say "# reach here because this is a child process"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78279/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122860370
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,42 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+selectTimeout <- function() {
+ if (is.null(parallel:::selectChildren(timeout = 0))) {
+ # Wait a socket connection indefinitely if there are no workers running.
+ NULL
--- End diff --
https://stat.ethz.ch/R-manual/R-devel/library/base/html/socketSelect.html
> NULL means wait indefinitely.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78517 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78517/testReport)** for PR 18320 at commit [`53e00d7`](https://github.com/apache/spark/commit/53e00d70871355e11b19f99c71fa34a92008ab27).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
This also happens i Mac to me. My assumption is, `mcfork` itself might be fine (opening a pipe between parent and child is okay) but when the child exits, it looks failed to return the resources properly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123443750
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -47,9 +79,11 @@ while (TRUE) {
close(inputCon)
--- End diff --
Added.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122687051
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
@felixcheung, I tested with the change below:
```diff
port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
+Sys.sleep(5L)
inputCon <- socketConnection(
port = port, blocking = TRUE, open = "rb", timeout = connectionTimeout)
outputCon <- socketConnection(
```
It looks fine. Does this deal with your concern?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
Hmm I'm not sure - I'm pretty sure the session / spark context is already initialized when this test is run and changing the setting here does it affect the existing daemon process already running?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the issue:
https://github.com/apache/spark/pull/18320
Thanks ! LGTM. Lets also wait to see if @felixcheung has anything more
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78148 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78148/testReport)** for PR 18320 at commit [`52c8abf`](https://github.com/apache/spark/commit/52c8abf9551e126f75ef0aa0a042f1ebd13e8d47).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/18320
Does it fail by running just gapply and nothing else?
From what you have found in your investigations and the code you pointed to, I suspect this isn't limited to gapply.
I think this PR so only works around the problem. I am concern that an user can also run into this issue.
An naive approach might be to change `park.sparkr.use.daemon` inside gapply when it is called, but I suspect that only shifts the problem around, and it might fail then with other methods that shuffles or calls UDFs. If a long running demon process is the problem, either we find and fix the leak (close the pipe, socket etc) or we put a count on the number of execution and re-cycle the demon process periodically before this leak becomes fatal.
thought?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78518 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78518/testReport)** for PR 18320 at commit [`6db1f61`](https://github.com/apache/spark/commit/6db1f61f28754e1caa888960eb1e7472dd2ff6d4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78286 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78286/testReport)** for PR 18320 at commit [`8b48274`](https://github.com/apache/spark/commit/8b48274dc565dc5c6722e983c55494b0067bda72).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78517/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
(I tested the current state with Mac OS for now).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78523/testReport)** for PR 18320 at commit [`c5fe347`](https://github.com/apache/spark/commit/c5fe3478298d992a7e91515651179d10563af445).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Mac OS
- [ ] jobs with many stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] jobs with long stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] NULL when we expect it - https://github.com/apache/spark/pull/18320#issuecomment-309940653
- [ ] sleep before connection - https://github.com/apache/spark/pull/18320#discussion_r122687051
- [ ] full tests
- [ ] dapply/gapply described in the PR
CentOS
- [ ] jobs with many stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] jobs with long stages - https://github.com/apache/spark/pull/18320#issuecomment-310312766
- [ ] NULL when we expect it - https://github.com/apache/spark/pull/18320#issuecomment-309940653
- [ ] sleep before connection - https://github.com/apache/spark/pull/18320#discussion_r122687051
- [ ] full tests
- [ ] dapply/gapply described in the PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123682838
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,51 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
--- End diff --
cool
could you also test without these sleep (trying to see if there's any race condition)
```
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
##### Sys.sleep(3L)
# Send no data
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
###### Sys.sleep(3L)
# Send custom data
parallel:::sendMaster("arbitrary")
parallel:::sendMaster(123)
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
####### Sys.sleep(3L)
# Send explicit singal.
parallel:::mcexit(0L, send = -1)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
####### Sys.sleep(10L)
# Check if `readChild` waits
parallel:::mcexit(0L, send = -1)
}
}
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78279 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78279/testReport)** for PR 18320 at commit [`4d9469b`](https://github.com/apache/spark/commit/4d9469b419570dd91cc73c944b3c8460b94a4021).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123416771
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
--- End diff --
If I understanding correctly, children (from `parallel:::selectChildren()`) as integer only indicates the list of fork/child process. But it does not indicate the child process "exited"?
When we are in a loop checking every second (ie. selectTimeout == 1), it sounds to me like socketSelect could return in one of the following ways:
- socket available for reading (ready == TRUE)
- socket not available for reading, it's dead or exiting (ready == FALSE)
- socket not available for reading, but it's not ready to connect because say things are running slow or the host is busy etc, and it has effectively been just timed out since selectTimeout (ready == FALSE)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122630135
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Terminate R workers in the parent process.
+ finishedChildren <- parallel:::selectChildren()
--- End diff --
ah got it.
could you add some comment in the code to document the behavior?
also I wonder if delay in connecting back to the JVM will now cause the code to abort prematurely?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
@felixcheung, I double checked that the current status solves the issue in both CentOS and Mac.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I think I am used to the symtoms here as I have tested many cases here. Let me try to organise and send a post to dev mailing list.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78518 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78518/testReport)** for PR 18320 at commit [`6db1f61`](https://github.com/apache/spark/commit/6db1f61f28754e1caa888960eb1e7472dd2ff6d4).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Yes, there is still the issue and this only fixes (avoid) the test failure. I believe running the codes should reproduce the issue for both Mac and CentOS.
What I don't get it, when the number of fork executions is not many, this is not reproduced (even sometimes increasing pipes were not observed sometimes). The number of the pipes decrease in a certain condition (it did not look related with time but some events).
The issue with `gapply` is exposed and found now as it invokes many forks via `daemon.R` but I guess this issue might still exist for all other APIs executing R native function with this daemon. I gave a shot to resolve the root cause within `daemon.R` with several tries but I could not make it.
Root cause is:
With a terminal executing `watch -n 0.01 "lsof -c R | wc -l"`
With another terminal:
```r
for(i in 0:200) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
tools::pskill(Sys.getpid(), tools::SIGUSR1)
parallel:::mcexit(0L)
}
}
```
The number of opened pipes just keep increasing. I double checked the processes and sockets are closed via `netstats` and `ps`.
We need to resolve this one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
(I tested the current state with Mac OS for now).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78447 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78447/testReport)** for PR 18320 at commit [`22837ca`](https://github.com/apache/spark/commit/22837ca9939c9a95bbd2312e101b83e5e191c069).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon closed the pull request at:
https://github.com/apache/spark/pull/18320
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Sure, I will test this one CentOS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123722811
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
Definitely.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78203 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78203/testReport)** for PR 18320 at commit [`74deb23`](https://github.com/apache/spark/commit/74deb2341ba2aabd58f280b5f7e463b6cad78c41).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123633470
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,40 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
--- End diff --
Sure, let me take a look and be back soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I also tested the current state on CentOS for sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78499 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78499/testReport)** for PR 18320 at commit [`9446c4a`](https://github.com/apache/spark/commit/9446c4a0e813b3e85673cb5db2c42fb003a28c59).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78282/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78286/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78281 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78281/testReport)** for PR 18320 at commit [`f3f57e4`](https://github.com/apache/spark/commit/f3f57e46868e66b8f50268910c1eff494638059d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78510/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122843971
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,30 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it sends a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent sends the signal to children every second or right before
+ # launching other worker children from the following new socket connection.
+ #
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ finishedChildren <- parallel:::selectChildren(timeout = 0)
--- End diff --
This does not block at all. I tested this - https://github.com/apache/spark/pull/18320#discussion_r122605437 for sure. Let me double check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78208 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78208/testReport)** for PR 18320 at commit [`99245fb`](https://github.com/apache/spark/commit/99245fb655cde49831737cbe5da78eaea55a51dc).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78284 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78284/testReport)** for PR 18320 at commit [`d6f0ff2`](https://github.com/apache/spark/commit/d6f0ff275abd3b7641210427eea955c9f0ea8d86).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78208/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122605188
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
--- End diff --
The logic here can be tested as below:
With a terminal (terminal A) running the command as below:
```bash
nc -l 7777
```
with another terminal (terminal B) running the codes below in R shell.
```R
inputCon <- socketConnection(port = "7777", open = "rb", blocking = TRUE)
print(socketSelect(list(inputCon), timeout = 1))
```
This prints `FALSE` as it does not have any data first.
In terminal A, if any character is typed first, this returns `TRUE` in terminal B with another execution.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
Meanwhile, let me try to find the root cause.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78447 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78447/testReport)** for PR 18320 at commit [`22837ca`](https://github.com/apache/spark/commit/22837ca9939c9a95bbd2312e101b83e5e191c069).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123716354
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,55 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of children sent data to the parent are returned below. The children
+ # send a custom exit code to the parent after being exited and the parent tries
+ # to terminate them only if they sent the exit code.
+ children <- parallel:::selectChildren(timeout = 0)
+
+ if (is.integer(children)) {
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is the exit code that indicates an exited child.
+ if (unserialize(data) == exitCode) {
+ # If so, we terminate this child.
+ tools::pskill(child, tools::SIGUSR1)
+ }
+ }
+ })
+ } else if (is.null(children)) {
+ # If it is NULL, there are no such children. Waits indefinitely for a socket connecion.
+ selectTimeout <- NULL
--- End diff --
It returns `NULL` if there are no children or there are no children that have data available?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122861395
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,42 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+selectTimeout <- function() {
+ if (is.null(parallel:::selectChildren(timeout = 0))) {
--- End diff --
Multiple children tests can be done as below:
copy and paste this instead
```r
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
Sys.sleep(3L)
parallel:::mcexit(0L)
}
}
print("TRUE - the timeout was reached")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
Sys.sleep(4L)
print("PID - child is exited (but not terminated)")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
invisible(lapply(parallel:::selectChildren(timeout = 0), function(x) { tools::pskill(x, tools::SIGUSR1) }))
Sys.sleep(1L)
print("NULL - if there are no children")
print("parallel:::selectChildren(timeout = 0):")
print(parallel:::selectChildren(timeout = 0))
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78510 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78510/testReport)** for PR 18320 at commit [`53e00d7`](https://github.com/apache/spark/commit/53e00d70871355e11b19f99c71fa34a92008ab27).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78148/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78278/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123701851
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,51 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
--- End diff --
I also tested
```r
kill <- function(children) {
lapply(children, function(child) {
data <- parallel:::readChild(child)
if (is.raw(data)) {
if (unserialize(data) == -1) {
print(paste("child PID:", child, "and parent will kill given:", unserialize(data)))
tools::pskill(child, tools::SIGUSR1)
} else {
print(paste("child PID:", child, "and sent a data:", unserialize(data)))
}
} else {
print(paste("child PID:", child, "and sent a PID:", data))
}
})
}
for(i in 0:1000) {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
##### Sys.sleep(3L)
# Send no data
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
###### Sys.sleep(3L)
# Send custom data
parallel:::sendMaster("arbitrary")
parallel:::sendMaster(123)
parallel:::mcexit(0L)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
####### Sys.sleep(3L)
# Send explicit singal.
parallel:::mcexit(0L, send = -1)
} else {
p <- parallel:::mcfork()
if (inherits(p, "masterProcess")) {
####### Sys.sleep(10L)
# Check if `readChild` waits
parallel:::mcexit(0L, send = -1)
}
}
}
}
c <- parallel:::selectChildren(timeout = 0)
if (!is.null(c)) {
print(paste("Killing", c))
invisible(kill(parallel:::selectChildren(timeout = 0)))
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18320
**[Test build #78513 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78513/testReport)** for PR 18320 at commit [`53e00d7`](https://github.com/apache/spark/commit/53e00d70871355e11b19f99c71fa34a92008ab27).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122848957
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
--- End diff --
Yea, I got your point and was taking a look. I think probably feasible via `parallel:::children`. I will give a shot to deal with this here and be back.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Terminate R's worker processes in the p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18320
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r123687096
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -30,8 +30,54 @@ port <- as.integer(Sys.getenv("SPARKR_WORKER_PORT"))
inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
+# Waits indefinitely for a socket connecion by default.
+selectTimeout <- NULL
+
+# Exit code that children send to the parent to indicate they exited.
+exitCode <- 1
+
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = selectTimeout)
+
+ # Note that the children should be terminated in the parent. If each child terminates
+ # itself, it appears that the resource is not released properly, that causes an unexpected
+ # termination of this daemon due to, for example, running out of file descriptors
+ # (see SPARK-21093). Therefore, the current implementation tries to retrieve children
+ # that are exited (but not terminated) and then sends a kill signal to terminate them properly
+ # in the parent.
+ #
+ # There are two paths that it attempts to send a signal to terminate the children in the parent.
+ #
+ # 1. Every second if any socket connection is not available and if there are child workers
+ # running.
+ # 2. Right after a socket connection is available.
+ #
+ # In other words, the parent attempts to send the signal to the children every second if
+ # any worker is running or right before launching other worker children from the following
+ # new socket connection.
+
+ # Only the process IDs of exited children are returned and the termination is attempted below.
+ children <- parallel:::selectChildren(timeout = 0)
+ if (is.integer(children)) {
+ # If it is PIDs, there are workers exited but not terminated. Attempts to terminate them
+ # by setting SIGUSR1.
+ lapply(children, function(child) {
+ # This data should be raw bytes if any data was sent from this child.
+ # Otherwise, this returns the PID.
+ data <- parallel:::readChild(child)
+ if (is.raw(data)) {
+ # This checks if the data from this child is -1 that indecides an exited child.
--- End diff --
`indecides` -> `indicates`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18320: [SPARK-21093][R] Avoid mcfork in R's daemon in gapply/ga...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18320
I was thinking we should not work around for now if this can't be fixed within `daemon.R` and it is not practically problematic which we don't know yet.
Let try to deal with`daemon.R`. Probably, the best way is to fix this problem together. I will be back after thinking more and taking a look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18320: [SPARK-21093][R] Terminate R's worker processes i...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18320#discussion_r122855545
--- Diff: R/pkg/inst/worker/daemon.R ---
@@ -31,7 +31,15 @@ inputCon <- socketConnection(
port = port, open = "rb", blocking = TRUE, timeout = connectionTimeout)
while (TRUE) {
- ready <- socketSelect(list(inputCon))
+ ready <- socketSelect(list(inputCon), timeout = 1)
--- End diff --
BTW, the old behaviour was simply to hang and wait there indefinitely.
https://stat.ethz.ch/R-manual/R-devel/library/base/html/socketSelect.html
> numeric or NULL. Time in seconds to wait for a socket to become available; NULL means wait indefinitely.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org