fightBoxing opened a new pull request, #4195:
URL: https://github.com/apache/amoro/pull/4195
Fixes #4172.
## Problem
When a table's optimizing process reaches the post-execution phase — all
tasks
are `SUCCESS` and the code is about to transition the table to `COMMITTING` —
a transient failure of `TableRuntime#beginCommitting()` (e.g. DB lock wait
timeout in the underlying `UPDATE table_runtime`) causes `invokeConsistency`
to roll back the in-memory status. The process stays `RUNNING` but the table
status remains `*_OPTIMIZING`, **forever, until AMS is restarted**. The issue
reporter confirmed exactly this symptom.
Root cause: there is no code path that retries or re-drives
\`beginCommitting()\` when it transiently fails. \`OptimizingCommitExecutor\`
only schedules when the status is already \`COMMITTING\`, so the stuck state
is invisible to it.
## Approach (A1: narrow, targeted self-heal)
Leverage the existing \`TableRuntimeRefreshExecutor\`, which already scans
every
table periodically. At the end of \`execute()\`, detect the stuck pattern and
re-drive the transition:
\`\`\`
process != null
&& process.getStatus() == RUNNING
&& tableRuntime.getOptimizingStatus() != COMMITTING
&& tableRuntime.getOptimizingStatus().isProcessing()
&& process.allTasksPrepared()
\`\`\`
When all conditions hold we call \`tableRuntime.beginCommitting()\`. On
success the normal \`handleTableChanged\` → \`OptimizingCommitExecutor\` path
resumes. On failure we log and retry on the next refresh cycle (≤ 1 minute
by default).
## Why this location
- \`TableRuntimeRefreshExecutor\` already does \`enabled() == true\` for
every
table — no new thread pool, no new scheduling mechanism.
- \`OptimizingCommitExecutor\` is intentionally kept as-is; it is only
notified
via the existing \`handleTableChanged\` event once the self-heal succeeds.
- \`OptimizingQueue.acceptResult()\` hot path (concurrent task completion) is
**not** changed — we only expose an existing read method.
## Changes
| File | Change |
|---|---|
| \`OptimizingProcess.java\` | Add \`boolean allTasksPrepared()\` to the
interface. |
| \`OptimizingQueue.java\` | \`TableOptimizingProcess.allTasksPrepared()\`
becomes \`@Override public\` and lock-protected for cross-thread visibility. |
| \`TableRuntimeRefreshExecutor.java\` | New private
\`tryHealStuckCommitting()\` invoked at the end of \`execute()\`. |
| \`TestDefaultOptimizingService.java\` | New regression test
\`testRefreshExecutorHealsStuckRunningProcess\` that uses reflection to
simulate the stuck state, then verifies \`execute()\` transitions back to
\`COMMITTING\`. |
## Verification
Target test passes locally:
\`\`\`
mvn -pl amoro-ams -am test \
-Dtest='TestDefaultOptimizingService#testRefreshExecutorHealsStuckRunningProcess'
\
-Dsurefire.failIfNoSpecifiedTests=false -DskipITs
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
\`\`\`
Self-heal path logs are observable during the test:
\`\`\`
WARN TableRuntimeRefresher: ...detected stuck RUNNING optimizing process
(processId=..., status=MINOR_OPTIMIZING): all tasks have succeeded but the
table never transitioned to COMMITTING. Self-healing by re-driving
beginCommitting() (issue #4172).
\`\`\`
## Risk
- No change to concurrent hot paths; only one additional idempotent read
(\`allTasksPrepared\`) per refresh cycle, short-circuited early for
tables that are not processing.
- Worst case if the underlying DB stays unhealthy: self-heal keeps logging
at \`WARN\` and retrying every refresh cycle — exactly the same cost as
before plus one cheap read. Strictly better than today's
"stuck until restart".
- Opened as **draft** first to let CI confirm, then I'll flip to ready
for review if green.
--
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]
