rdblue commented on a change in pull request #1823:
URL: https://github.com/apache/iceberg/pull/1823#discussion_r552092876



##########
File path: 
aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java
##########
@@ -100,10 +104,11 @@ protected void doRefresh() {
   protected void doCommit(TableMetadata base, TableMetadata metadata) {
     String newMetadataLocation = writeNewMetadata(metadata, currentVersion() + 
1);
     boolean exceptionThrown = true;
-    Table glueTable = getGlueTable();
-    checkMetadataLocation(glueTable, base);
-    Map<String, String> properties = prepareProperties(glueTable, 
newMetadataLocation);
     try {
+      lock(newMetadataLocation);

Review comment:
       Because this locks in the `try` block, the finally will be called when 
the lock fails. I think that's correct for cleaning up the metadata location, 
but the lock release will currently fail because the lock isn't held by this 
thread. That will cause the lock failure exception to get replaced by the 
unlock failure.
   
   I think the solution is to not throw exceptions in release. I'll comment on 
that below.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to