Skip to content

Commit

Permalink
Handle FileAlreadyExistsException in fixDeltaLog
Browse files Browse the repository at this point in the history
  • Loading branch information
scottsand-db committed Apr 27, 2023
1 parent c05d0c5 commit 4c70285
Showing 1 changed file with 14 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ public void write(
prevFileName
);
if (prevEntry.isPresent() && !prevEntry.get().complete) {
// This will never throw a java.nio.file.FileAlreadyExistsException for N-1.json
// (which would be confusing for the writer trying to write N.json).
fixDeltaLog(fs, prevEntry.get());
} else {
if (!fs.exists(prevPath)) {
Expand Down Expand Up @@ -334,6 +336,12 @@ protected void fixDeltaLogPutCompleteDbEntry(ExternalCommitEntry entry) throws I
/**
* Method for assuring consistency on filesystem according to the external cache.
* Method tries to rewrite TransactionLog entry from temporary path if it does not exist.
*
* Should never throw a java.nio.file.FileAlreadyExistsException:
* - if N.json already exists, we either don't copy T(N) -> N.json, or we swallow the
* FileAlreadyExistsException
* - we won't receive a FileAlreadyExistsException when writing to the external cache, as
* overwrite will be true
*/
private void fixDeltaLog(FileSystem fs, ExternalCommitEntry entry) throws IOException {
if (entry.complete) {
Expand All @@ -351,7 +359,12 @@ private void fixDeltaLog(FileSystem fs, ExternalCommitEntry entry) throws IOExce
fixDeltaLogPutCompleteDbEntry(entry);
LOG.info("fixed {}", entry.fileName);
return;
} catch(Throwable e) {
} catch (java.nio.file.FileAlreadyExistsException e) {
LOG.info("{}:", e.getClass().getSimpleName(), e);
copied = true;

// Never re-throw a FileAlreadyExistsException
} catch (IOException e) {
LOG.info("{}:", e.getClass().getSimpleName(), e);
if (retry >= 3) {
throw e;
Expand Down

2 comments on commit 4c70285

@scottsand-db
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems incomplete. writeCopyTempFile(fs, entry.absoluteTempPath(), resolvedPath); also does a copy ...

@allisonport-db
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think writeCopyTempFile doesn't need a wrapper since we never propagate an exception from there (catch (Throwable e))

Please sign in to comment.