pan3793 commented on pull request #35076: URL: https://github.com/apache/spark/pull/35076#issuecomment-1008497632
@mridulm thank you for your response! > did you mean `volatile` in this context instead? Having said that, all state is modified with the AppShufflePartitionInfo locked - so would be within that critical section. Oops, sorry, I mean `volatile`. Another question here, the `synchronized(appInfo)` ensures the visibility of `appInfo` itself, but what about the member variables without `volatile` of `appInfo`? > The `IOException` being thrown in that method is when we are unable to close the stream - in this case, it is a close of the `fd`. While possible in theory, usually it would point to other more severe issues outside of what spark can deal with. But you are right, it does log and ignore failures if close fails. Then I think if `#closeAllFilesAndDeleteIfNeeded` throws IOE, we need to mark this partition merge failed. > The truncate is actually a best case effort to clean up excess disk space usage. Then it has a chance that the actual file size is greater than metadata, i.e. because of a hardware issue, the disk becomes read-only when doing truncate(in this case it will throw IOE?). But since we can always trust the content before the 'good' position, for the case that file length greater than the 'good' position still can be treated as good merged data? -- 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-unsubscr...@spark.apache.org 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
