nandorKollar commented on code in PR #4934:
URL: https://github.com/apache/polaris/pull/4934#discussion_r3497984240
##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/CommitTransactionEventTest.java:
##########
@@ -273,4 +280,79 @@ private CommitTransactionRequest
generateCommitTransactionRequest(
updateRequirements,
List.of(new MetadataUpdate.SetProperties(Map.of(propertyName,
"value2"))))));
}
+
+ @Test
+ void testCommitTransactionCleansUpMetadataOnFailure(@TempDir Path tempDir) {
+ String location = tempDir.toAbsolutePath().toUri().toString();
+ if (location.endsWith("/")) {
+ location = location.substring(0, location.length() - 1);
+ }
+
+ // Create TestServices with a spy that will fail on
updateEntitiesPropertiesIfNotChanged
+ // but only AFTER initial setup (table creation) succeeds.
+ AtomicBoolean shouldFail = new AtomicBoolean(false);
+ TestServices testServices =
+ TestServices.builder()
+ .config(
+ Map.of(
+ "ALLOW_INSECURE_STORAGE_TYPES",
+ "true",
+ "SUPPORTED_CATALOG_STORAGE_TYPES",
+ List.of("FILE")))
+ .metaStoreManagerDecorator(
+ msm -> {
+ org.apache.polaris.core.persistence.PolarisMetaStoreManager
spy =
+ Mockito.spy(msm);
+ Mockito.doAnswer(
+ invocation -> {
+ if (shouldFail.get()) {
+ return new EntitiesResult(
+
BaseResult.ReturnStatus.ENTITY_CANNOT_BE_RESOLVED,
+ "simulated CAS failure");
+ }
+ return invocation.callRealMethod();
+ })
+ .when(spy)
+ .updateEntitiesPropertiesIfNotChanged(Mockito.any(),
Mockito.any());
+ return spy;
+ })
+ .build();
+
+ createCatalogAndNamespace(testServices, Map.of(), location);
+
+ String table1Name = "cleanup-table-1";
+ String table2Name = "cleanup-table-2";
+ createTable(testServices, table1Name, location);
+ createTable(testServices, table2Name, location);
+
+ // Count metadata files before the failing transaction
+ long metadataFilesBefore = countMetadataFiles(tempDir);
+
+ // Now enable the CAS failure and attempt a commitTransaction
+ shouldFail.set(true);
+ assertThatThrownBy(
+ () ->
+ testServices
+ .restApi()
+ .commitTransaction(
+ catalog,
+ generateCommitTransactionRequest(false, table1Name,
table2Name),
+ IDEMPOTENCY_KEY,
+ testServices.realmContext(),
+ testServices.securityContext()))
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageContaining("Transaction commit failed");
+
+ // After the failed transaction, no new metadata files should remain (they
were cleaned up)
+ long metadataFilesAfter = countMetadataFiles(tempDir);
+ assertThat(metadataFilesAfter).isEqualTo(metadataFilesBefore);
Review Comment:
nit: Probably matching the exact set of paths before and after is more
accurate.
--
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]
