From 3186d339c0661681505299b1603ace2f343e9cbf Mon Sep 17 00:00:00 2001 From: James Faulkner Date: Mon, 6 Jul 2026 17:00:02 +0100 Subject: [PATCH 1/2] feat: add structured metrics for deleteIfUnchanged outcomes in GlueSync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the flat 'deleted' outcome tag on drop/rename events with a typed DeleteOutcome enum (DELETED, SKIPPED, NOT_FOUND) so each code path through the optimistic delete guard is independently observable. - deleteIfUnchanged now returns DeleteOutcome instead of void, letting callers record the correct metric without a switch statement - SKIPPED (guard fired — identity params diverged) is now a distinct metric outcome rather than silently counting as success/deleted - NOT_FOUND is consolidated: both the getTable and the TOCTOU deleteTable EntityNotFoundException paths return NOT_FOUND from deleteIfUnchanged, removing the separate catch block in onDropTable - Rename operations now emit operation=rename_table instead of alter_table, making them independently filterable in dashboards - 128 tests, all passing Co-Authored-By: Claude Sonnet 4.6 --- .../gluesync/listener/ApiaryGlueSync.java | 16 ++--- .../listener/metrics/MetricConstants.java | 8 ++- .../listener/service/GlueTableService.java | 41 +++++++++++-- .../gluesync/listener/ApiaryGlueSyncTest.java | 59 +++++++++++++++++-- .../service/GlueTableServiceTest.java | 43 ++++++++++---- 5 files changed, 134 insertions(+), 33 deletions(-) diff --git a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSync.java b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSync.java index 7681b395..60a144d6 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSync.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSync.java @@ -186,15 +186,9 @@ public void onDropTable(DropTableEvent event) throws MetaException { } Table table = event.getTable(); try { - glueTableService.deleteIfUnchanged(table); + GlueTableService.DeleteOutcome outcome = glueTableService.deleteIfUnchanged(table); metricService.incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - metricService.recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "deleted"); - } catch (EntityNotFoundException e) { - log.info(table + " table doesn't exist in glue catalog"); - metricService.recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "not_found"); - if (throwExceptions) { - throw wrap(e); - } + metricService.recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, outcome.metricOutcome()); } catch (Exception e) { log.error("Failed drop table {}.{} in glue", table.getDbName(), table.getTableName(), e); metricService.incrementCounter(MetricConstants.LISTENER_TABLE_FAILURE); @@ -293,10 +287,10 @@ private void doRenameOperation(Table oldTable, Table newTable) { long startTime = System.currentTimeMillis(); glueTableService.create(newTable); gluePartitionService.copyPartitions(newTable, gluePartitionService.getPartitions(oldTable)); - glueTableService.deleteIfUnchanged(oldTable); - metricService.incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - metricService.recordEvent(MetricConstants.ALTER_TABLE, MetricConstants.RESULT_SUCCESS, "renamed"); + GlueTableService.DeleteOutcome outcome = glueTableService.deleteIfUnchanged(oldTable); long duration = System.currentTimeMillis() - startTime; + metricService.incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); + metricService.recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, outcome.metricOutcome()); metricService.recordDuration(MetricConstants.LISTENER_TABLE_RENAME_DURATION, duration); log.info("{} glue table rename to {} finished in {}ms", oldTable.getTableName(), newTable.getTableName(), duration); } diff --git a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java index 618181a7..c65cadc2 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java @@ -38,14 +38,18 @@ public class MetricConstants { public static final String RESULT_FAILURE = "failure"; public static final String RESULT_IGNORED = "ignored"; - public static final String OUTCOME_CREATED = "created"; - public static final String OUTCOME_UPDATED = "updated"; + public static final String OUTCOME_CREATED = "created"; + public static final String OUTCOME_UPDATED = "updated"; + public static final String OUTCOME_DELETED = "deleted"; + public static final String OUTCOME_NOT_FOUND = "not_found"; + public static final String OUTCOME_DELETE_SKIPPED = "delete_skipped"; public static final String CREATE_DATABASE = "create_database"; public static final String DROP_DATABASE = "drop_database"; public static final String CREATE_TABLE = "create_table"; public static final String DROP_TABLE = "drop_table"; public static final String ALTER_TABLE = "alter_table"; + public static final String RENAME_TABLE = "rename_table"; public static final String ADD_PARTITION = "add_partition"; public static final String DROP_PARTITION = "drop_partition"; public static final String ALTER_PARTITION = "alter_partition"; diff --git a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java index 307c657c..aa1e9634 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java @@ -34,6 +34,32 @@ import com.amazonaws.services.glue.model.ValidationException; public class GlueTableService { + + /** + * Outcome of a {@link #deleteIfUnchanged} call, used by callers to record the appropriate metric. + * + */ + public enum DeleteOutcome { + DELETED("deleted"), + SKIPPED("delete_skipped"), + NOT_FOUND("not_found"); + + private final String metricOutcome; + + DeleteOutcome(String metricOutcome) { + this.metricOutcome = metricOutcome; + } + + public String metricOutcome() { + return metricOutcome; + } + } + private static final Logger log = LoggerFactory.getLogger(GlueTableService.class); // HMS parameters checked before a delete to detect if the table slot was overwritten by a concurrent operation private static final String[] IDENTITY_PARAMS = { "transient_lastDdlTime", "metadata_location" }; @@ -112,7 +138,7 @@ public void delete(Table table) { * delete will be skipped, leaving an orphan in Glue. An orphan is intentionally preferred over * incorrectly deleting a table that has since been overwritten by a rename. */ - public void deleteIfUnchanged(Table table) { + public DeleteOutcome deleteIfUnchanged(Table table) { Map hmsParams = table.getParameters() != null ? table.getParameters() : Collections.emptyMap(); com.amazonaws.services.glue.model.Table glueTable; try { @@ -123,17 +149,24 @@ public void deleteIfUnchanged(Table table) { .getTable(); } catch (EntityNotFoundException e) { log.info("{}.{} table not found in glue catalog, nothing to delete", table.getDbName(), table.getTableName()); - return; + return DeleteOutcome.NOT_FOUND; } Map glueParams = glueTable.getParameters() != null ? glueTable.getParameters() : Collections.emptyMap(); for (String key : IDENTITY_PARAMS) { if (paramChanged(key, hmsParams, glueParams, table)) { - return; + return DeleteOutcome.SKIPPED; } } - delete(table); + try { + delete(table); + } catch (EntityNotFoundException e) { + log.info("{}.{} table deleted from glue catalog between guard check and delete", + table.getDbName(), table.getTableName()); + return DeleteOutcome.NOT_FOUND; + } + return DeleteOutcome.DELETED; } private boolean paramChanged(String key, Map hmsParams, Map glueParams, Table table) { diff --git a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java index 458ca2c5..48c65dc6 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java @@ -482,7 +482,7 @@ public void onAlterHiveTable_RenameTable() throws MetaException { BatchCreatePartitionRequest batchCreatePartitionRequest = batchCreatePartitionRequestCaptor.getValue(); verify(glueClient).deleteTable(deleteTableRequestCaptor.capture()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.ALTER_TABLE, MetricConstants.RESULT_SUCCESS, "renamed"); + verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETED); verify(metricService).recordDuration(eq(MetricConstants.LISTENER_TABLE_RENAME_DURATION), anyLong()); DeleteTableRequest deleteTableRequest = deleteTableRequestCaptor.getValue(); @@ -711,7 +711,7 @@ public void onDropTable() throws MetaException { verify(glueClient).deleteTable(deleteTableRequestCaptor.capture()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "deleted"); + verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETED); assertThat(deleteTableRequestCaptor.getValue().getDatabaseName(), is(gluePrefix + dbName)); assertThat(deleteTableRequestCaptor.getValue().getName(), is(tableName)); } @@ -721,16 +721,63 @@ public void onDropTableThatDoesntExistInGlue() throws MetaException { DropTableEvent event = mock(DropTableEvent.class); when(event.getStatus()).thenReturn(true); when(event.getTable()).thenReturn(simpleHiveTable(simpleSchema(), simplePartitioning())); - when(glueClient.getTable(any())).thenReturn(new GetTableResult().withTable(new com.amazonaws.services.glue.model.Table())); - when(glueClient.deleteTable(any())).thenThrow(new EntityNotFoundException("")); + when(glueClient.getTable(any())).thenThrow(new EntityNotFoundException("")); glueSync.onDropTable(event); - verify(glueClient).deleteTable(any()); - verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "not_found"); + verify(glueClient, never()).deleteTable(any()); + verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); + verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_NOT_FOUND); verifyNoMoreInteractions(metricService); } + @Test + public void onDropTable_skipsDeleteWhenGuardFires() throws MetaException { + DropTableEvent event = mock(DropTableEvent.class); + when(event.getStatus()).thenReturn(true); + Table hmsTable = simpleHiveTable(simpleSchema(), simplePartitioning()); + hmsTable.putToParameters("transient_lastDdlTime", "100"); + when(event.getTable()).thenReturn(hmsTable); + when(glueClient.getTable(any())).thenReturn( + new GetTableResult().withTable(new com.amazonaws.services.glue.model.Table() + .withParameters(ImmutableMap.of("transient_lastDdlTime", "999")))); + + glueSync.onDropTable(event); + + verify(glueClient, never()).deleteTable(any()); + verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); + verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETE_SKIPPED); + verifyNoMoreInteractions(metricService); + } + + @Test + public void onAlterHiveTable_RenameTable_skipsOldTableDeleteWhenGuardFires() throws MetaException { + AlterTableEvent event = mock(AlterTableEvent.class); + when(event.getStatus()).thenReturn(true); + + Table oldTable = simpleHiveTable(simpleSchema(), simplePartitioning()); + oldTable.setTableName("table2"); + oldTable.putToParameters("transient_lastDdlTime", "100"); + when(event.getOldTable()).thenReturn(oldTable); + + Table newTable = simpleHiveTable(simpleSchema(), simplePartitioning()); + newTable.setTableName("table2_new"); + when(event.getNewTable()).thenReturn(newTable); + + when(glueClient.getPartitions(any())).thenReturn(new GetPartitionsResult().withPartitions(Collections.emptyList())); + // Glue has a different DDL time — guard fires, old table slot not deleted + when(glueClient.getTable(any())).thenReturn( + new GetTableResult().withTable(new com.amazonaws.services.glue.model.Table() + .withParameters(ImmutableMap.of("transient_lastDdlTime", "999")))); + + glueSync.onAlterTable(event); + + verify(glueClient, never()).deleteTable(any()); + verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); + verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETE_SKIPPED); + verify(metricService).recordDuration(eq(MetricConstants.LISTENER_TABLE_RENAME_DURATION), anyLong()); + } + @Test public void onAlterHiveTable_RenameOperationFailureIsMetered() throws MetaException { AlterTableEvent event = mock(AlterTableEvent.class); diff --git a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java index 4cd5f014..b0e9f35e 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java @@ -15,6 +15,8 @@ */ package com.expediagroup.apiary.extensions.gluesync.listener.service; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -62,8 +64,9 @@ public void setUp() { public void deleteIfUnchanged_deletesWhenAllPropertiesMatch() { when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null)); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + assertThat(outcome, is(GlueTableService.DeleteOutcome.DELETED)); verify(glueClient).deleteTable(any(DeleteTableRequest.class)); } @@ -71,8 +74,9 @@ public void deleteIfUnchanged_deletesWhenAllPropertiesMatch() { public void deleteIfUnchanged_skipsWhenLastDdlTimeChanged() { when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, "1751385999", null)); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } @@ -82,8 +86,9 @@ public void deleteIfUnchanged_skipsWhenHiveTableReplacedByIceberg() { when(glueClient.getTable(any(GetTableRequest.class))) .thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json")); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } @@ -92,8 +97,10 @@ public void deleteIfUnchanged_skipsWhenMetadataLocationDiverges() { when(glueClient.getTable(any(GetTableRequest.class))) .thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v2.metadata.json")); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json")); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged( + hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json")); + assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } @@ -101,28 +108,42 @@ public void deleteIfUnchanged_skipsWhenMetadataLocationDiverges() { public void deleteIfUnchanged_skipsWhenHmsHasMetadataLocationButGlueDoesNot() { when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null)); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json")); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged( + hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json")); + assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } @Test - public void deleteIfUnchanged_skipsWhenTableNotFoundInGlue() { + public void deleteIfUnchanged_notFoundWhenTableAbsentDuringGet() { when(glueClient.getTable(any(GetTableRequest.class))).thenThrow(new EntityNotFoundException("not found")); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + assertThat(outcome, is(GlueTableService.DeleteOutcome.NOT_FOUND)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } + @Test + public void deleteIfUnchanged_notFoundWhenTableDeletedBetweenGuardAndDelete() { + when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null)); + when(glueClient.deleteTable(any(DeleteTableRequest.class))).thenThrow(new EntityNotFoundException("not found")); + + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + + assertThat(outcome, is(GlueTableService.DeleteOutcome.NOT_FOUND)); + } + @Test public void deleteIfUnchanged_deletesWhenBothParamMapsAreNull() { when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResultNullParams()); Table table = hmsTable(LOCATION, null, null); table.setParameters(null); - service.deleteIfUnchanged(table); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(table); + assertThat(outcome, is(GlueTableService.DeleteOutcome.DELETED)); verify(glueClient).deleteTable(any(DeleteTableRequest.class)); } @@ -132,8 +153,9 @@ public void deleteIfUnchanged_skipsWhenHmsParamsNullButGlueHasParams() { Table table = hmsTable(LOCATION, null, null); table.setParameters(null); - service.deleteIfUnchanged(table); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(table); + assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } @@ -141,8 +163,9 @@ public void deleteIfUnchanged_skipsWhenHmsParamsNullButGlueHasParams() { public void deleteIfUnchanged_skipsWhenGlueParamsNullButHmsHasParams() { when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResultNullParams()); - service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); + assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED)); verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class)); } From d51b78142a565ed798e282765cf3a5e1a072a8cb Mon Sep 17 00:00:00 2001 From: James Faulkner Date: Tue, 7 Jul 2026 08:58:41 +0100 Subject: [PATCH 2/2] feat: add DeleteOutcome metrics for GlueTableService.deleteIfUnchanged Surfaces distinct metric outcomes (deleted, delete_skipped, not_found, concurrent_delete) for the optimistic delete guard so alerting can distinguish a skipped guard from a successful delete. Co-Authored-By: Claude Sonnet 4.6 --- .../gluesync/listener/metrics/MetricConstants.java | 7 ++----- .../gluesync/listener/service/GlueTableService.java | 9 ++++++--- .../gluesync/listener/ApiaryGlueSyncTest.java | 10 +++++----- .../listener/service/GlueTableServiceTest.java | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java index c65cadc2..f56cccb8 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java @@ -38,11 +38,8 @@ public class MetricConstants { public static final String RESULT_FAILURE = "failure"; public static final String RESULT_IGNORED = "ignored"; - public static final String OUTCOME_CREATED = "created"; - public static final String OUTCOME_UPDATED = "updated"; - public static final String OUTCOME_DELETED = "deleted"; - public static final String OUTCOME_NOT_FOUND = "not_found"; - public static final String OUTCOME_DELETE_SKIPPED = "delete_skipped"; + public static final String OUTCOME_CREATED = "created"; + public static final String OUTCOME_UPDATED = "updated"; public static final String CREATE_DATABASE = "create_database"; public static final String DROP_DATABASE = "drop_database"; diff --git a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java index aa1e9634..de679fdd 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java @@ -41,13 +41,16 @@ public class GlueTableService { *
  • {@code DELETED} — the Glue table was deleted.
    • *
  • {@code SKIPPED} — the delete was suppressed because identity params diverged, indicating * the Glue slot was overwritten by a concurrent operation.
    • - *
  • {@code NOT_FOUND} — the table did not exist in Glue; nothing to delete.
    • + *
  • {@code NOT_FOUND} — the table did not exist in Glue when checked; nothing to delete.
    • + *
  • {@code DELETED_CONCURRENTLY} — the table existed at guard-check time but was deleted by a + * concurrent operation before the delete call completed (TOCTOU race).
    • * */ public enum DeleteOutcome { DELETED("deleted"), SKIPPED("delete_skipped"), - NOT_FOUND("not_found"); + NOT_FOUND("not_found"), + DELETED_CONCURRENTLY("concurrent_delete"); private final String metricOutcome; @@ -164,7 +167,7 @@ public DeleteOutcome deleteIfUnchanged(Table table) { } catch (EntityNotFoundException e) { log.info("{}.{} table deleted from glue catalog between guard check and delete", table.getDbName(), table.getTableName()); - return DeleteOutcome.NOT_FOUND; + return DeleteOutcome.DELETED_CONCURRENTLY; } return DeleteOutcome.DELETED; } diff --git a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java index 48c65dc6..b2e1fe2a 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java @@ -482,7 +482,7 @@ public void onAlterHiveTable_RenameTable() throws MetaException { BatchCreatePartitionRequest batchCreatePartitionRequest = batchCreatePartitionRequestCaptor.getValue(); verify(glueClient).deleteTable(deleteTableRequestCaptor.capture()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETED); + verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, "deleted"); verify(metricService).recordDuration(eq(MetricConstants.LISTENER_TABLE_RENAME_DURATION), anyLong()); DeleteTableRequest deleteTableRequest = deleteTableRequestCaptor.getValue(); @@ -711,7 +711,7 @@ public void onDropTable() throws MetaException { verify(glueClient).deleteTable(deleteTableRequestCaptor.capture()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETED); + verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "deleted"); assertThat(deleteTableRequestCaptor.getValue().getDatabaseName(), is(gluePrefix + dbName)); assertThat(deleteTableRequestCaptor.getValue().getName(), is(tableName)); } @@ -727,7 +727,7 @@ public void onDropTableThatDoesntExistInGlue() throws MetaException { verify(glueClient, never()).deleteTable(any()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_NOT_FOUND); + verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "not_found"); verifyNoMoreInteractions(metricService); } @@ -746,7 +746,7 @@ public void onDropTable_skipsDeleteWhenGuardFires() throws MetaException { verify(glueClient, never()).deleteTable(any()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETE_SKIPPED); + verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "delete_skipped"); verifyNoMoreInteractions(metricService); } @@ -774,7 +774,7 @@ public void onAlterHiveTable_RenameTable_skipsOldTableDeleteWhenGuardFires() thr verify(glueClient, never()).deleteTable(any()); verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS); - verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, MetricConstants.OUTCOME_DELETE_SKIPPED); + verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, "delete_skipped"); verify(metricService).recordDuration(eq(MetricConstants.LISTENER_TABLE_RENAME_DURATION), anyLong()); } diff --git a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java index b0e9f35e..7c4dd25c 100644 --- a/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java +++ b/hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java @@ -126,13 +126,13 @@ public void deleteIfUnchanged_notFoundWhenTableAbsentDuringGet() { } @Test - public void deleteIfUnchanged_notFoundWhenTableDeletedBetweenGuardAndDelete() { + public void deleteIfUnchanged_deletedConcurrentlyWhenTableDeletedBetweenGuardAndDelete() { when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null)); when(glueClient.deleteTable(any(DeleteTableRequest.class))).thenThrow(new EntityNotFoundException("not found")); GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null)); - assertThat(outcome, is(GlueTableService.DeleteOutcome.NOT_FOUND)); + assertThat(outcome, is(GlueTableService.DeleteOutcome.DELETED_CONCURRENTLY)); } @Test