You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/10/24 16:28:45 UTC
[GitHub] [spark] swamirishi opened a new pull request, #38377: SPARK-40901. Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
swamirishi opened a new pull request, #38377:
URL: https://github.com/apache/spark/pull/38377
### What changes were proposed in this pull request?
Parsing the configuration value of the config value of spark.driver.log.dfsDir using Hadoop FS Path & extracting path from the URI should fix the problem
### Why are the changes needed?
Currently when one passes the Absolute URI to configured filesystem, the code would fail while trying to copy the local log file to the filesystem directory path.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit test cases
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1310773116
@mridulm when can this be merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1311068364
Merged to master.
Thanks for fixing this @swamirishi !
Thanks for the review @dongjoon-hyun :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1299613189
Makes sense ... why not simply `val dfsLogFile = new Path(rootDir, appId + DRIVER_LOG_FILE_SUFFIX)` instead btw ?
I am trying to see if I am missing anything here ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1297907890
QQ: Any details for the query [above](https://github.com/apache/spark/pull/38377#pullrequestreview-1153485176) ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #38377: SPARK-40901. Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1003551847
##########
core/src/test/scala/org/apache/spark/util/logging/DriverLoggerSuite.scala:
##########
@@ -59,15 +60,17 @@ class DriverLoggerSuite extends SparkFunSuite with LocalSparkContext {
sc.stop()
assert(!driverLogsDir.exists())
- val dfsFile = FileUtils.getFile(sc.getConf.get(DRIVER_LOG_DFS_DIR).get,
- app_id + DriverLogger.DRIVER_LOG_FILE_SUFFIX)
- assert(dfsFile.exists())
- assert(dfsFile.length() > 0)
+ val dfsFile = new Path(sc.getConf.get(DRIVER_LOG_DFS_DIR).get +
+ "/" + app_id + DriverLogger.DRIVER_LOG_FILE_SUFFIX)
+ val dfsFileStatus = dfsFile.getFileSystem(sc.hadoopConfiguration).getFileStatus(dfsFile)
+
+ assert(dfsFileStatus.isFile)
+ assert(dfsFileStatus.getLen > 0)
}
private def getSparkContext(): SparkContext = {
val conf = new SparkConf()
- conf.set(DRIVER_LOG_DFS_DIR, rootDfsDir.getAbsolutePath())
+ conf.set(DRIVER_LOG_DFS_DIR, "file://" + rootDfsDir.getAbsolutePath())
conf.set(DRIVER_LOG_PERSISTTODFS, true)
conf.set(SparkLauncher.SPARK_MASTER, "local")
Review Comment:
Can we add a test specifically for this ? So that existing tests continue to check for local path ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1005223413
##########
core/src/test/scala/org/apache/spark/util/logging/DriverLoggerSuite.scala:
##########
@@ -65,12 +65,42 @@ class DriverLoggerSuite extends SparkFunSuite with LocalSparkContext {
assert(dfsFile.length() > 0)
}
+ test("driver logs are persisted locally and synced to dfs when log dir is absolute URI") {
Review Comment:
Could you add a test prefix?
```scala
- test("driver logs are persisted locally and synced to dfs when log dir is absolute URI") {
+ test("SPARK-40901: Driver logs are persisted locally and synced to dfs when log dir is absolute URI") {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1301454290
I am not sure I follow - can you give an example ?
Based on what the PR is doing, I would expect both to be equivalent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1302754420
> If we do necessarily need the fully qualified path, we can use `fileSystem.resolvePath(new Path ...)` instead.
FileUtils.get().absolutePath() would resolve file path based on the System.property("user.dir") in the case of while fileSystem.resolvePath will throw FilenotfoundException
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on a diff in pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1014524002
##########
core/src/main/scala/org/apache/spark/util/logging/DriverLogger.scala:
##########
@@ -142,7 +142,7 @@ private[spark] class DriverLogger(conf: SparkConf) extends Logging {
threadpool = ThreadUtils.newDaemonSingleThreadScheduledExecutor("dfsSyncThread")
threadpool.scheduleWithFixedDelay(this, UPLOAD_INTERVAL_IN_SECS, UPLOAD_INTERVAL_IN_SECS,
TimeUnit.SECONDS)
- logInfo(s"Started driver log file sync to: ${dfsLogFile}")
+ logInfo(s"Started driver log file sync to: ${dfsLogFile.toString}")
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on a diff in pull request #38377: SPARK-40901. Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1003569481
##########
core/src/test/scala/org/apache/spark/util/logging/DriverLoggerSuite.scala:
##########
@@ -59,15 +60,17 @@ class DriverLoggerSuite extends SparkFunSuite with LocalSparkContext {
sc.stop()
assert(!driverLogsDir.exists())
- val dfsFile = FileUtils.getFile(sc.getConf.get(DRIVER_LOG_DFS_DIR).get,
- app_id + DriverLogger.DRIVER_LOG_FILE_SUFFIX)
- assert(dfsFile.exists())
- assert(dfsFile.length() > 0)
+ val dfsFile = new Path(sc.getConf.get(DRIVER_LOG_DFS_DIR).get +
+ "/" + app_id + DriverLogger.DRIVER_LOG_FILE_SUFFIX)
+ val dfsFileStatus = dfsFile.getFileSystem(sc.hadoopConfiguration).getFileStatus(dfsFile)
+
+ assert(dfsFileStatus.isFile)
+ assert(dfsFileStatus.getLen > 0)
}
private def getSparkContext(): SparkContext = {
val conf = new SparkConf()
- conf.set(DRIVER_LOG_DFS_DIR, rootDfsDir.getAbsolutePath())
+ conf.set(DRIVER_LOG_DFS_DIR, "file://" + rootDfsDir.getAbsolutePath())
conf.set(DRIVER_LOG_PERSISTTODFS, true)
conf.set(SparkLauncher.SPARK_MASTER, "local")
Review Comment:
Sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1306060490
@dongjoon-hyun @mridulm Any other changes I need to make on this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] asfgit closed pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
URL: https://github.com/apache/spark/pull/38377
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1297930510
> QQ: Any details for the query [above](https://github.com/apache/spark/pull/38377#pullrequestreview-1153485176) ?
Currently all of the logs can be only written to the default filesytem defined by fs.Defaultfs job config. One wouldn't be able to write the logs to other filesystem directly without changing the property as absolute urls are not supported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1290595534
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1303865771
> Two points:
>
> * spark.driver.log.dfsDir is typically expected to be a path to hdfs - so resolving it relative to current working directory does not make sense
> * If `rootDir` is referencing local filesystem, it will get resolved relative to local fs by the `fileSystem` for `rootDir`
This can be done using makeQualified API instead of resolvePath.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on a diff in pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1009717064
##########
core/src/test/scala/org/apache/spark/util/logging/DriverLoggerSuite.scala:
##########
@@ -65,12 +65,42 @@ class DriverLoggerSuite extends SparkFunSuite with LocalSparkContext {
assert(dfsFile.length() > 0)
}
+ test("driver logs are persisted locally and synced to dfs when log dir is absolute URI") {
Review Comment:
Done
##########
core/src/test/scala/org/apache/spark/util/logging/DriverLoggerSuite.scala:
##########
@@ -44,7 +45,6 @@ class DriverLoggerSuite extends SparkFunSuite with LocalSparkContext {
test("driver logs are persisted locally and synced to dfs") {
val sc = getSparkContext()
-
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1005223160
##########
core/src/test/scala/org/apache/spark/util/logging/DriverLoggerSuite.scala:
##########
@@ -44,7 +45,6 @@ class DriverLoggerSuite extends SparkFunSuite with LocalSparkContext {
test("driver logs are persisted locally and synced to dfs") {
val sc = getSparkContext()
-
Review Comment:
Shall we revert this empty line removal?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1303007426
Two points:
* spark.driver.log.dfsDir is typically expected to be a path to hdfs - so resolving it relative to current working directory does not make sense
* If `rootDir` is referencing local filesystem, it will get resolved relative to local fs by the `fileSystem` for `rootDir`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1014469439
##########
core/src/main/scala/org/apache/spark/util/logging/DriverLogger.scala:
##########
@@ -142,7 +142,7 @@ private[spark] class DriverLogger(conf: SparkConf) extends Logging {
threadpool = ThreadUtils.newDaemonSingleThreadScheduledExecutor("dfsSyncThread")
threadpool.scheduleWithFixedDelay(this, UPLOAD_INTERVAL_IN_SECS, UPLOAD_INTERVAL_IN_SECS,
TimeUnit.SECONDS)
- logInfo(s"Started driver log file sync to: ${dfsLogFile}")
+ logInfo(s"Started driver log file sync to: ${dfsLogFile.toString}")
Review Comment:
nit: remove `.toString`
##########
core/src/main/scala/org/apache/spark/util/logging/DriverLogger.scala:
##########
@@ -126,13 +126,13 @@ private[spark] class DriverLogger(conf: SparkConf) extends Logging {
throw new RuntimeException(s"${rootDir} does not exist." +
s" Please create this dir in order to persist driver logs")
}
- val dfsLogFile: String = FileUtils.getFile(rootDir, appId
- + DriverLogger.DRIVER_LOG_FILE_SUFFIX).getAbsolutePath()
+ val dfsLogFile: Path = fileSystem.makeQualified(new Path(rootDir, appId
+ + DriverLogger.DRIVER_LOG_FILE_SUFFIX))
Review Comment:
Looking at it more, given all resolution is related to `fileSystem`, we dont need to either resolve or qualify it technically - the only reason to do so is for the log message below: so I am fine with this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1301028102
> Makes sense ... why not simply `val dfsLogFile = new Path(rootDir, appId + DRIVER_LOG_FILE_SUFFIX)` instead btw ? If we do necessarily need the fully qualified path, we can use `fileSystem.resolvePath(new Path ...)` instead. I am trying to see if I am missing anything here ...
Currently the logs are written based on the current working directory of the job when the path is relative. So the output path would go to the home directory of the user in the default filesystem instead of the CWD which would be the output log directory configured in the job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] swamirishi commented on a diff in pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #38377:
URL: https://github.com/apache/spark/pull/38377#discussion_r1016480277
##########
core/src/main/scala/org/apache/spark/util/logging/DriverLogger.scala:
##########
@@ -126,13 +126,13 @@ private[spark] class DriverLogger(conf: SparkConf) extends Logging {
throw new RuntimeException(s"${rootDir} does not exist." +
s" Please create this dir in order to persist driver logs")
}
- val dfsLogFile: String = FileUtils.getFile(rootDir, appId
- + DriverLogger.DRIVER_LOG_FILE_SUFFIX).getAbsolutePath()
+ val dfsLogFile: Path = fileSystem.makeQualified(new Path(rootDir, appId
+ + DriverLogger.DRIVER_LOG_FILE_SUFFIX))
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on pull request #38377: [SPARK-40901][CORE] Unable to store Spark Driver logs with Absolute Hadoop based URI FS Path
Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #38377:
URL: https://github.com/apache/spark/pull/38377#issuecomment-1310907480
I wanted to give @dongjoon-hyun a chance to take a look if he has bandwidth, given he had reviewed it.
Will merge it later tonight if there are no other comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org