viirya commented on a change in pull request #26312: URL: https://github.com/apache/spark/pull/26312#discussion_r506020849
########## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileFormatWriter.scala ########## @@ -281,6 +281,10 @@ object FileFormatWriter extends Logging { } catch { case e: FetchFailedException => throw e + case f: FileAlreadyExistsException => Review comment: It is not a rare case actually. We had customer encountered this issue in production jobs occasionally. There are also other people reporting this issue. It is remarkable because this `FileAlreadyExistsException` cannot be handled by user code. It is happened internally in Spark, and users have no way to deal with it. Image your production jobs fail after 10+ hours because of `FileAlreadyExistsException`. I believe it is worth failing fast the production jobs. It sounds really unreliable to rely on retry to handle a rare race that causes `FileAlreadyExistsException`. What if later attempts cannot success too? Seems to me a production job won't rely on that and needs address this issue seriously. In most of cases this can be handled by catching `FileAlreadyExistsException` and retrying. In rare case if any, there is rare race that can cause `FileAlreadyExistsException` and it cannot make change? It sounds pretty rare to me, and even if it is really happens we can retry the call of the library/service to solve it, so I'm not sure if we should sacrifice the cases seen in production for a rare case. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org