[ENG-45879] Classify control-plane "table is deleted" as a skip, not an API failure

[suryadanny]

Summary

When a table is deleted control-plane side, POST /v1/community/initialize-tables returns HTTP 200 with a per-table error string:

Table can be skipped as it is deleted

That is an advisory — "there is nothing to upload for this table" — not a failure. But the call site treated any non-blank per-table error as API_FAILURE_USER_ERROR and logged at ERROR, twice per table per cycle (once at the call site, once inside incrementTableMetadataProcessingFailureCounter).

Found while investigating PD Q043G44USWZC7F: an org with 198 of 199 tables deleted produced 9,528 counted "failures" in ~9.5 hours on a completely healthy extractor — Running, 1/1, restartCount: 0, lakeView_table_discovery_success_total incrementing +12/hr throughout. Nothing was wrong with the extractor, the network, or the credentials.

Two concrete harms:

1. Misdirected triage. API_FAILURE_USER_ERROR reads as connectivity or credentials — the same reason code is used for real 401/403s from OnehouseApiClient.emmitApiErrorMetric. The natural first move is to go chase an expired API token, when in fact no HTTP call failed.
2. The metric isn't actionable. lakeView_table_metadata_processing_failure_total{metadata_upload_failure_reason="API_FAILURE_USER_ERROR"} could not distinguish "the customer deleted their tables" (no action) from "our API is rejecting us" (page someone).

Changes

Before	After
..._processing_failure{reason="API_FAILURE_USER_ERROR"}	lakeView_table_skipped{table_skip_reason="DELETED"}
ERROR ×2 per table per cycle	INFO ×1

• MetricsConstants — new TableSkipReasons { DELETED }, deliberately separate from MetadataUploadFailureReasons so the failure counter keeps meaning "something went wrong".
• MetadataExtractorUtils.isTableDeletedError(String) — classifies the advisory, placed alongside the existing getMetadataExtractorFailureReason helper.
• LakeViewExtractorMetrics.incrementTableSkippedCounter(...) — new lakeView_table_skipped counter tagged table_skip_reason. Deliberately does not log (unlike the failure method, which logs internally — that's the source of the second ERROR line); the caller logs at INFO with table context.
• TableMetadataUploaderService — branch on the classification before the failure path.

Kept a counter rather than going silent: lakeView_table_skipped{table_skip_reason="DELETED"} would have answered the whole investigation in one query.

Technical details

Real errors are unaffected. Only the deleted-advisory is reclassified; every other per-table error still increments API_FAILURE_USER_ERROR and logs at ERROR. There is a regression test asserting exactly that.

Behavior otherwise unchanged. The skip branch continues exactly as the old error branch did, so control flow and the batch return value are untouched.

Known limitation — matched on message text. InitializeTableMetricsCheckpointResponse.InitializeSingleTableMetricsCheckpointResponse carries only a free-text error field; there is no structured error/status code to switch on. A control-plane reword could silently break the match, so I used two distinctive markers rather than the full sentence and documented the constraint in the javadoc. The durable fix is control-plane side: a structured errorCode / skipReason on that response, at which point isTableDeletedError becomes a code check. Happy to follow up if the API team wants to add one.

Testing

./gradlew :lakeview:test — 42 tests across the three affected classes, all passing:

• MetadataExtractorUtilsTest (16) — the verbatim production string; the wrapped form as it reaches the call site; case-insensitivity; a reworded prefix; plus negative cases asserting 401/403/Internal server error and deletion-adjacent-but-real failures are not reclassified; and null/empty.
• TableMetadataUploaderServiceTest (13) — two new: a deleted table increments the skip counter, never the failure counter, and is never uploaded; a real error still counts as API_FAILURE_USER_ERROR and never as a skip.
• LakeViewExtractorMetricsTest (13) — the new counter's tag set, and that it never touches the failure counter.

./gradlew spotlessCheck passes.

⚠️ Pre-existing unrelated test failure on main: AsyncHttpClientWithRetryTest.testMakeRequestWithRetryIOException fails (1 of 322). Confirmed pre-existing by stashing this branch's changes and re-running it against clean origin/main — it fails identically there. Not introduced here, but it will make CI's clean check red until it's fixed separately.

Noted, not changed

A real per-table init error still lets the batch report success — the error path continues without adding a future, and allMatch on an empty stream returns true. That's correct for the deleted case (nothing to do), but arguably wrong for real errors. Left alone because fixing it changes behavior beyond classification; worth its own ticket.

Related ticket

• ENG-45879 — this bug
• ENG-45878 — RCA of PD Q043G44USWZC7F that surfaced it
• knowledge-base#241 — runbook branch so on-call doesn't misread the reason code before this ships

🤖 Generated with Claude Code

[nimahajan]

Task linked: ENG-45878 RCA: metadataExtractorNotUploadingInstants — Q043G44USWZC7F
Task linked: ENG-45879 [LakeView] Benign "table is deleted" response logged as ERROR + counted as API_FAILURE_USER_ERROR (alert noise)