[GitHub] [spark] HeartSaVioR commented on a change in pull request #31495: [SPARK-34383][SS] Optimize WAL commit phase via reducing cost of filesystem operations

[GitBox <gi...@apache.org>]

HeartSaVioR commented on a change in pull request #31495:
URL: https://github.com/apache/spark/pull/31495#discussion_r595562205



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/CheckpointFileManager.scala
##########
@@ -277,7 +277,6 @@ class FileSystemBasedCheckpointFileManager(path: Path, hadoopConf: Configuration
       fs.delete(path, true)
     } catch {
       case e: FileNotFoundException =>
-        logInfo(s"Failed to delete $path as it does not exist")

Review comment:
       I'm quoting my comment:
   
   > FileSystemBasedCheckpointFileManager.delete logs INFO message when the file doesn't exist. FileContextBasedCheckpointFileManager.delete doesn't log any message though.
   >
   > Probably OK to remove INFO message on FileSystemBasedCheckpointFileManager.delete as FileSystemBasedCheckpointFileManager is used as a fail-back of FileContextBasedCheckpointFileManager (especially Spark logs warn message when failing back) and FileContextBasedCheckpointFileManager covers the major usage.
   
   We decided to "blindly" call `delete()` without checking existence in prior, as file system will do it instead and we can save one op call. Leaving this log would provide false-alarm, as it's "expected" even if there's no file, so we have to delete the log line.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org