datahub-gc: KeyError in SoftDeletedEntitiesCleanup._process_futures masks delete failures and aborts the stage
Describe the bug
SoftDeletedEntitiesCleanup._process_futures raises KeyError whenever any hard delete fails, which aborts the whole soft-deleted cleanup stage and discards the underlying exception. The failure-reporting path is itself the thing that breaks, so the real cause never reaches the report.
The dict is rebound to only the not-done futures, and then indexed with the done ones:
def _process_futures(self, futures: Dict[Future, Urn]) -> Dict[Future, Urn]:
done, not_done = wait(futures, return_when=FIRST_COMPLETED)
futures = {future: urn for future, urn in futures.items() if future in not_done} # only not_done survive
for future in done:
self._print_report()
if future.exception():
self.report.failure(
title="Failed to delete entity",
message="Failed to delete entity",
context=futures[future].urn(), # KeyError: `future` is in `done`, so it was just filtered out
exc=future.exception(),
)futures[future] can only ever succeed for a future that is not in done, but the loop iterates exactly over done. So the lookup raises KeyError for the first future that carries an exception. Because _process_futures is called from cleanup_soft_deleted_entities without a guard, the KeyError propagates and ends the stage.
Nothing happens on the happy path, which is why this hides: it only triggers once a delete fails, and then it converts one entity-level failure into a stage-level abort plus a useless error message.
To Reproduce
Run the datahub-gc source with soft_deleted_entities_cleanup.enabled: true using a token that lacks DELETE_ENTITY, so every delete raises.
source:
type: datahub-gc
config:
cleanup_expired_tokens: false
truncate_indices: false
dataprocess_cleanup:
enabled: false
execution_request_cleanup:
enabled: false
soft_deleted_entities_cleanup:
enabled: true
retention_days: 90Observed on acryl-datahub 1.7.0 against DataHub GMS 1.7.0:
ERROR {datahub.ingestion.source.gc.datahub_gc:178} - While trying to cleanup soft deleted entities
While trying to cleanup soft deleted entities : <Future at 0x7f80ac1c24d0 state=finished raised OperationalError>
'num_soft_deleted_entity_found': 1115,
'num_soft_deleted_retained_due_to_age': 1043,
'num_soft_deleted_entity_processed': 114,
'num_soft_deleted_entity_removal_started': 71,
'num_hard_deleted': 0,
'failures': [{'message': 'While trying to cleanup soft deleted entities ',
'context': ["<class 'KeyError'>: <Future ... raised OperationalError>"]}],Two things to note in that report. The KeyError is reported instead of the OperationalError that actually occurred, and the stage stopped after 114 of 1,115 found entities rather than continuing past the failures.
The real cause turned out to be an authorization gap, which we only found by calling the same endpoint the source calls, by hand:
POST /entities?action=delete -> HTTP 403
"User is unauthorized to delete entity: urn:li:chart:(looker,dashboard_elements....)"Expected behavior
A delete that fails should be reported against its own URN with its own exception, and the run should carry on with the remaining futures. Concretely, capture the mapping before filtering:
def _process_futures(self, futures: Dict[Future, Urn]) -> Dict[Future, Urn]:
done, not_done = wait(futures, return_when=FIRST_COMPLETED)
done_urns = {future: futures[future] for future in done}
futures = {future: urn for future, urn in futures.items() if future in not_done}
for future in done:
self._print_report()
if future.exception():
self.report.failure(
title="Failed to delete entity",
message="Failed to delete entity",
context=done_urns[future].urn(),
exc=future.exception(),
)Separately, it would help a lot if a 403 from delete_entity were surfaced as a distinct, actionable failure. An operator reading this report has no way to tell "the token cannot delete" apart from "the backend is unwell", and the privilege requirement is not mentioned in the datahub-gc docs.
Version
acryl-datahub1.7.0 (CLI), DataHub GMS 1.7.0, Python 3.11- Present unchanged on
masteras of this writing.
Source: datahub-project/datahub