Skip to content

Conversation

@tsreaper
Copy link
Contributor

When the job runs to checkpoint N, if the user recovers from an old checkpoint (such as checkpoint N-5), the sink of the current FTS will cause a manifest corruption because duplicate files may be committed.

We should avoid such corruption, and the storage should be robust enough.

} catch (Throwable e) {
throw new RuntimeException(
"File deletion conflicts detected! Give up committing compact changes.", e);
throw new RuntimeException("File deletion conflicts detected! Give up committing.", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can ignore exception from mergeManifestEntries here.
We can explain there are concurrent writing and print to change detail.

readAllEntriesFromChangedPartitions(
latestSnapshotId, appendChanges, compactChanges));
entriesToCheck.addAll(appendChanges);
noConflictsOrFail(entriesToCheck);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to explain in the exception: When there will be conflict and what should be done

Copy link
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@JingsongLi JingsongLi merged commit 33896da into apache:master Sep 22, 2022
JingsongLi pushed a commit that referenced this pull request Sep 22, 2022
@tsreaper tsreaper deleted the append-commit-check branch October 8, 2022 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants