RussellSpitzer commented on code in PR #4612:
URL: https://github.com/apache/iceberg/pull/4612#discussion_r856264100
##########
core/src/main/java/org/apache/iceberg/BaseRewriteManifests.java:
##########
@@ -251,13 +251,15 @@ private void performRewrite(List<ManifestFile>
currentManifests) {
}
}
- private void validateDeletedManifests(Set<ManifestFile> currentManifests) {
+ private void validateDeletedManifests(Set<ManifestFile> currentManifests,
long currentSnapshotId) {
// directly deleted manifests must be still present in the current snapshot
deletedManifests.stream()
.filter(manifest -> !currentManifests.contains(manifest))
.findAny()
.ifPresent(manifest -> {
- throw new ValidationException("Manifest is missing: %s",
manifest.path());
+ throw new ValidationException("Cannot apply RewriteManifests result,
" +
Review Comment:
I think in this message we want to make clear that this is not a corrupt
table issue and that it is most likely the result of a concurrent manifest
operation. Maybe something like
"Cannot commit RewriteManifests; manifests that existed at the beginning of
this rewrite have already been removed by another operation. Manifest %s, which
would be replaced by this operation, has already been removed."?
I'm not sure the snapshot ID really helps unless you also pass the snapshot
that the operation started on, but even with that information I'm not sure a
User can do much.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
