[GitHub] [iceberg] ChristinaTech commented on a diff in pull request #7198: AWS: Check commit status after failed commit if AWS client performed retries

["ChristinaTech (via GitHub)" <gi...@apache.org>]

ChristinaTech commented on code in PR #7198:
URL: https://github.com/apache/iceberg/pull/7198#discussion_r1155044079


##########
aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogCommitFailure.java:
##########
@@ -123,6 +128,74 @@ public void testConcurrentModificationExceptionDoesNotCheckCommitStatus() {
     Assert.assertEquals("No new metadata files should exist", 2, metadataFileCount(ops.current()));
   }
 
+  @Test
+  public void testCheckCommitStatusAfterRetries() {
+    String namespace = createNamespace();
+    String tableName = createTable(namespace);
+    TableIdentifier tableId = TableIdentifier.of(namespace, tableName);
+
+    GlueTableOperations spyOps =
+        Mockito.spy((GlueTableOperations) glueCatalog.newTableOps(tableId));
+    GlueCatalog spyCatalog = Mockito.spy(glueCatalog);
+    Mockito.doReturn(spyOps).when(spyCatalog).newTableOps(Mockito.eq(tableId));
+    Table table = spyCatalog.loadTable(tableId);
+
+    TableMetadata metadataV1 = spyOps.current();
+    simulateRetriedCommit(spyOps, true);
+    updateTable(table, spyOps);
+
+    Assert.assertNotEquals("Current metadata should have changed", metadataV1, spyOps.current());
+    Assert.assertTrue("Current metadata should still exist", metadataFileExists(spyOps.current()));
+    Assert.assertEquals(
+        "No new metadata files should exist", 2, metadataFileCount(spyOps.current()));
+  }
+
+  @Test
+  public void testNoRetryAwarenessCorruptsTable() {
+    // This test exists to replicate the issue the prior test validates the fix for
+    String namespace = createNamespace();
+    String tableName = createTable(namespace);
+    TableIdentifier tableId = TableIdentifier.of(namespace, tableName);
+
+    GlueTableOperations spyOps =
+        Mockito.spy((GlueTableOperations) glueCatalog.newTableOps(tableId));
+    GlueCatalog spyCatalog = Mockito.spy(glueCatalog);
+    Mockito.doReturn(spyOps).when(spyCatalog).newTableOps(Mockito.eq(tableId));
+    Table table = spyCatalog.loadTable(tableId);
+
+    simulateRetriedCommit(spyOps, false);

Review Comment:
   I agree, I will add something similar to that.



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org