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
