You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ouyangxiaochen <gi...@git.apache.org> on 2018/10/24 06:56:49 UTC
[GitHub] spark pull request #22813: [SPARK-25818][CORE] WorkDirCleanup should only re...
GitHub user ouyangxiaochen opened a pull request:
https://github.com/apache/spark/pull/22813
[SPARK-25818][CORE] WorkDirCleanup should only remove the directory at the beginning of t…
## What changes were proposed in this pull request?
The cleanup mechanism will clear all the eligible directories under SPARK_WORK_DIR. If the other configured paths are the same as the SPARK_WORK_DIR configuration, this will cause the file directories of other configuration items to be deleted by mistake. For example, the SPARK_LOCAL_DIRS and SPARK_WORK_DIR settings are the same.
We should add an another condition which start with 'app-' when removing the app-* directories in SPARK_WORK_DIR
## How was this patch tested?
manual test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ouyangxiaochen/spark SPARK-25818
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22813.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 #22813
----
commit cf8b30f64f8855ea1574fa47955a7ce42c7d0703
Author: ouyangxiaochen <ou...@...>
Date: 2018-10-24T06:48:24Z
WorkDirCleanup should only remove the directory at the beginning of the app-
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22813
@ouyangxiaochen . Sorry, but the use case sounds like a misconfiguration.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22813
I don't know what else goes in the work dir. It isn't valid to reuse it for anything else. Can you simply avoid using a work dir that is or has been used by something else?
The argument for making this change anyway is just that the code should delete just what it writes. But I am just not sure it can be something you rely on for correct behavior
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by ouyangxiaochen <gi...@git.apache.org>.
Github user ouyangxiaochen commented on the issue:
https://github.com/apache/spark/pull/22813
As far as I know, when a spark program is submitted to the cluster, a directory will be created under `SPARK_WORK_DIR`. The directory name consists of application, timestamp, and five-digit serial number. `WorkDirCleanUp `should only delete expired application directories. @srowen Could you tell me if there are other types of directories or files being created under `SPARK_WORK_DIR`? In addition to the application directory. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by ouyangxiaochen <gi...@git.apache.org>.
Github user ouyangxiaochen commented on the issue:
https://github.com/apache/spark/pull/22813
@dongjoon-hyun Yes, you can think so. So I want to solve this problem on the spark platform to reduce the risk of some misoperations of operation and maintenance engineer. `WorkDirCleanUp `is only responsible for cleaning up the directories that it generates.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22813: [SPARK-25818][CORE] WorkDirCleanup should only re...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22813
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22813
I just think that if you have engineers randomly writing and reading stuff in this dir, a bunch of other stuff goes wrong. This is not a problem that Spark can reasonably solve. Certainly, you have much bigger production problems if this level of discipline can't be enforced.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by ouyangxiaochen <gi...@git.apache.org>.
Github user ouyangxiaochen commented on the issue:
https://github.com/apache/spark/pull/22813
Yes, it happened in our real environment.
The scenario as follows:
Some disk corruption in the production cluster which is normal.
SPARK_LOCAL_DIRS = /data1/bigdata/spark/tmp, disk `data1 `is broken, the maintenance engineer modified `data1 `to `data2 ` or another. Unfortunately. the config SPARK_WORK_DIR = /data2/bigdata/spark/tmp, and then we start a `Thriftserver` process, some temporay folder will be created at the new config path which is the same as SPARK_WORK_DIR, but when the cleanup cycle time is reached, the folder created by `Thriftserver` will be removed by `WorkDirCleanUP`, so it will cause the Beeline and JDBC query to fail.
There is a very extreme situation that the user configures the operating system directory, which will cause a lot of trouble. So i think add this condition could reduce some unnecessary risks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/22813
IIUC it's not expected to share the SPARK_WORK_DIR with any other usage.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22813
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22813
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22813
I don't think that's a reasonable usage scenario. That said is there any harm to this change? would it ever miss cleaning something up that it should?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by ouyangxiaochen <gi...@git.apache.org>.
Github user ouyangxiaochen commented on the issue:
https://github.com/apache/spark/pull/22813
@srowen Your suggestion is very good, but sometimes the maintenance engineers have limited skills in this area. If they configures the operating system root directory as `SPARK_WORK_DIR` due to disk damage, it will bring a catastrophic accident. So, I think it is necessary to add this condition to avoid this production accidents.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22813: [SPARK-25818][CORE] WorkDirCleanup should only remove th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22813
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org