zhang-arvin opened a new pull request, #4432: URL: https://github.com/apache/amoro/pull/4432
## Root cause Since 0.9, both the normal close path and a failed commit routed through `completeProcess(false)`, so **disabling self-optimizing (or cancelling) was recorded as a process failure**. On top of that, when the in-memory `OptimizingProcess` was already gone (AMS restart / released process), the `table_process` row was never closed and stayed `RUNNING` forever: - `TableRuntimeRefreshExecutor.handleConfigChanged` only closed the **in-memory** process, and only when `getOptimizingProcess() != null`. Later refreshes did not retry, so an orphan row stayed `RUNNING`. - `DefaultOptimizingService.cancelProcess` returned `false` when the in-memory process was missing, but `TableController.cancelOptimizingProcess` still returned HTTP 200. - `OptimizingQueue...persistAndSetCompleted(false)` / `close()` called `completeProcess(false)`, which incremented `amoro_table_optimizing_process_failed_count_total` even for a user/config close. ## Changes - **Distinguish CLOSED from FAILED** — `DefaultTableRuntime.completeProcess` now takes the terminal `ProcessStatus`; `TableOptimizingMetrics.processComplete(type, success, failed, planTime)` only increments the failure counters for a genuine `FAILED` process. A CLOSED/CANCELED/KILLED process no longer counts as a failure. - **Close by process id from the store** — new `DefaultTableRuntime.closeProcessFromStore(processId)` (and `closeTrackedProcessFromStore()`) persist-close a non-terminal `table_process` row owned by the table and reset the table to `IDLE` when no in-memory process exists. - **Retry while disabled** — `TableRuntimeRefreshExecutor` calls the close on every refresh while self-optimizing stays disabled, so an orphan `RUNNING` row cannot survive. - **Cancel is no longer a silent no-op** — `DefaultOptimizingService.cancelProcess` falls back to closing the tracked row by id; `TableController.cancelOptimizingProcess` fails (non-200) when cancel did not happen. - **Null-safety** — `completeProcess` is now null-safe (previously NPE'd when the in-memory process was gone, as seen in `OptimizingQueue.releaseTable`). ## Tests `TestDefaultOptimizingService`: - disabling self-optimizing with a live RUNNING process → row becomes `CLOSED`, table `IDLE`, `process_failed_count` unchanged; - disabling self-optimizing when the in-memory process is already gone → orphan row becomes `CLOSED`, counter unchanged; - cancel-by-id when the in-memory process is gone → returns `true` and closes the row. `mvn -pl amoro-ams -am test -Dtest=TestDefaultOptimizingService` (33 tests) and the related `TestIcebergV3OptimizingPlanning`, `TestTableRuntimeRefreshExecutor`, `TestTableSummary*`, `TestOverviewManager`, `TestTableManagerQuery` suites pass; `spotless:check` passes. Fixes #4431. -- 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]
