RussellSpitzer commented on code in PR #9333:
URL: https://github.com/apache/iceberg/pull/9333#discussion_r1430079206
##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java:
##########
@@ -146,29 +147,37 @@ public void commit(TableMetadata base, TableMetadata
metadata) {
!metadata.properties().containsKey(TableProperties.WRITE_METADATA_LOCATION),
"Hadoop path-based tables cannot relocate metadata");
- String codecName =
- metadata.property(
- TableProperties.METADATA_COMPRESSION,
TableProperties.METADATA_COMPRESSION_DEFAULT);
- TableMetadataParser.Codec codec =
TableMetadataParser.Codec.fromName(codecName);
- String fileExtension = TableMetadataParser.getFileExtension(codec);
- Path tempMetadataFile = metadataPath(UUID.randomUUID().toString() +
fileExtension);
- TableMetadataParser.write(metadata,
io().newOutputFile(tempMetadataFile.toString()));
+ boolean versionCommitSuccess = false;
+ try{
+ String codecName =
+ metadata.property(
+ TableProperties.METADATA_COMPRESSION,
TableProperties.METADATA_COMPRESSION_DEFAULT);
+ TableMetadataParser.Codec codec =
TableMetadataParser.Codec.fromName(codecName);
+ String fileExtension = TableMetadataParser.getFileExtension(codec);
+ Path tempMetadataFile = metadataPath(UUID.randomUUID().toString() +
fileExtension);
+ TableMetadataParser.write(metadata,
io().newOutputFile(tempMetadataFile.toString()));
- int nextVersion = (current.first() != null ? current.first() : 0) + 1;
- Path finalMetadataFile = metadataFilePath(nextVersion, codec);
- FileSystem fs = getFileSystem(tempMetadataFile, conf);
+ //TODO: What if the variable versionId overflows?
+ long nextVersion = (current.first() != null ? current.first() : 0) + 1;
+ Path finalMetadataFile = metadataFilePath(nextVersion, codec);
+ FileSystem fs = getFileSystem(tempMetadataFile, conf);
- // this rename operation is the atomic commit operation
- renameToFinal(fs, tempMetadataFile, finalMetadataFile, nextVersion);
+ // this rename operation is the atomic commit operation
+ renameToFinal(fs, tempMetadataFile, finalMetadataFile, nextVersion);
- LOG.info("Committed a new metadata file {}", finalMetadataFile);
+ LOG.info("Committed a new metadata file {}", finalMetadataFile);
- // update the best-effort version pointer
- writeVersionHint(nextVersion);
+ // update the best-effort version pointer
+ versionCommitSuccess = writeVersionHint(nextVersion);
- deleteRemovedMetadataFiles(base, metadata);
+ deleteRemovedMetadataFiles(base, metadata);
- this.shouldRefresh = true;
+ this.shouldRefresh = true;
+ }catch (Throwable e){
Review Comment:
This is a bit over cautious, only after line 166 do we believe the commit
state is unknown, prior to this we know it was a faiulre
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
