Replace CLI_ASYNC_DDL with DDL_EXECUTION_MODE and DDL_ASYNC_WAIT_TIMEOUT

[apstndb]

Fixes #485.

Replace boolean CLI_ASYNC_DDL with driver-parity DDL_EXECUTION_MODE=SYNC|ASYNC|ASYNC_WAIT and DDL_ASYNC_WAIT_TIMEOUT (default 10s). --async selects ASYNC. SYNC remains the default.

Semantics

• SYNC waits for the Admin LRO and reports its actual result, including a completed failing LRO.
• ASYNC returns the accepted operation ID immediately. Later failure remains visible through SHOW OPERATION.
• ASYNC_WAIT reuses the existing DDL wait helper with one remaining wait budget across the initial GetOperation poll, later polls, and the between-poll wait. On budget expiry it returns the still-running operation ID as a successful asynchronous submission. Expiry cancels only the polling RPC and does not send CancelOperation for the accepted DDL.
• A terminal result already on the accepted operation (from UpdateDatabaseDdl or a preceding poll) is reported as-is, including when the wait budget is already zero.
• Caller/STATEMENT_TIMEOUT cancellation remains an error that includes the operation ID.
• A completed failing LRO remains a failure.
• Progress teardown, schema-generation invalidation, proto descriptors, and completed-DDL echo are unchanged.

CLI_ASYNC_DDL is removed (not a name-only alias). Migration: SET CLI_ASYNC_DDL = TRUE → SET DDL_EXECUTION_MODE = 'ASYNC'.

Review follow-up

• P1 on 1ad9423 (in-flight polls): 72718d829ac0082e422b43df1af648c7625c5078
• P1 on 81c15ff (zero-budget completed LRO failure): 744f217facb7bd4b6663d193ce6c8a665b22e561

Additive merge of origin/main 92f986d (#975) is included. Draft PR #947 was inspected for wait-loop cancel timing only and was not adopted.

Validation

• Isolated checkout based on origin/main after Synchronize SAVEPOINT replay cancellation before mock RPC release #975 (92f986d).
• Local full make check with Docker (exit 0), then make check-race (exit 0), before push.
• Coverage floor remains 80%.

  

[github-actions]

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

	main (92f986d)	#974 (1a22434)	+/-
Coverage	86.7%	86.8%	+0.1%
Code to Test Ratio	1:2.2	1:2.2	-0.1
Test Execution Time	2m1s	2m3s	+2s

Details

  |                     | main (92f986d) | #974 (1a22434) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          86.7% |          86.8% | +0.1% |
  |   Files             |            112 |            112 |     0 |
  |   Lines             |          10428 |          10486 |   +58 |
+ |   Covered           |           9051 |           9112 |   +61 |
- | Code to Test Ratio  |          1:2.2 |          1:2.2 |  -0.1 |
  |   Code              |          23309 |          23491 |  +182 |
+ |   Test              |          52940 |          53348 |  +408 |
- | Test Execution Time |           2m1s |           2m3s |   +2s |

Code coverage of files in pull request scope (90.8% → 91.7%, patch 85.0%)

Files	Coverage	+/-	Patch Coverage	Status
enums/enums.go	0.0%	0.0%	-	modified
internal/mycli/config.go	90.2%	+0.0%	100.0%	modified
internal/mycli/execute_ddl.go	86.9%	+8.1%	82.1%	modified
internal/mycli/system_variables.go	96.5%	0.0%	100.0%	modified
internal/mycli/var_defs.go	96.0%	+0.0%	100.0%	modified
internal/mycli/var_enum_handlers.go	88.1%	+0.1%	100.0%	modified
internal/mycli/var_handler.go	93.9%	+0.3%	100.0%	modified

---

Reported by octocov

[apstndb]

Reviewed 1ad9423229ec7c2363f0bfa23573fcb60cd4229d.

[P1] Apply the ASYNC_WAIT budget to in-flight polling RPCs. Runtime-reproduced at the poll call: the timer is only observed after op.Poll(ctx) returns. A slow/stalled first or later GetOperation therefore ignores DDL_ASYNC_WAIT_TIMEOUT and can wait until the much longer caller/statement deadline. In an independent overlay probe, the fake GetOperation waited for its RPC context to finish, ASYNC_WAIT was 20ms, and the outer caller guard was 500ms. The command waited for that 500ms guard and returned a DeadlineExceeded error instead of successful handoff of op-ddl.

Use one remaining wait budget across initial/subsequent polls as well as the between-poll wait. A deadline on the GetOperation request cancels only the polling RPC; it must never send CancelOperation for the accepted DDL. Distinguish budget expiry from caller cancellation/STATEMENT_TIMEOUT and a completed failing LRO. Add regression coverage with GetOperation actually blocked until context cancellation, plus caller-deadline precedence and the zero-budget boundary; the current timeout test only covers a immediately returning pending poll. Keep the existing poll helper and progress teardown.

Existing focused DDL RPC and wait-budget tests passed independently. The new slow-poll regression fails as described, while tracked files remain unchanged (a Go test overlay supplied the blocking fake).

Validation is also incomplete: the PR body lists short tests/lint/format/race and says full make check needs Docker. The accepted lane requires make check before every push. Set up/start Docker in the assigned environment, run the full gate with captured output and actual exit status, and report that evidence after the fix; passing hosted CI does not retroactively establish that local gate. Do not silently substitute short tests for it.

[cursor]

P1 addressed on 72718d829ac0082e422b43df1af648c7625c5078.

ASYNC_WAIT now applies one remaining wait budget to the initial GetOperation poll, later polls, and the between-poll wait. Budget expiry cancels only the polling RPC and hands off the still-running operation ID as a successful async submission; it never sends CancelOperation. Caller/STATEMENT_TIMEOUT cancellation and a completed failing LRO stay distinct.

Regression coverage: GetOperation blocked until context cancellation, caller-deadline precedence, and the zero-budget boundary.

Current PR HEAD 81c15fff24ba4e683ba8204d65f51a1b2904c13b is an additive merge of that fix with origin/main 92f986d (#975).

This replies to #974 (comment)

[apstndb]

Reviewed 81c15fff24ba4e683ba8204d65f51a1b2904c13b. The slow GetOperation regression is fixed and the new focused tests pass independently. The full Docker gate is now reported in the PR.

[P1] Preserve an already-completed LRO failure when the wait budget is zero. Runtime-reproduced through the existing Admin RPC fake: UpdateDatabaseDdl returns Done=true with FailedPrecondition: completed DDL failure, DDL_EXECUTION_MODE is ASYNC_WAIT, and DDL_ASYNC_WAIT_TIMEOUT is 0. ddlPollContext returns the budget sentinel before op.Poll can read the cached terminal result. finishWaitErr then calls formatAsyncDdlResult, yielding err=nil, DONE=true and an empty ERROR cell. A known failed DDL is therefore reported as success.

The budget controls waiting for a still-pending operation; it must not bypass a terminal result already received from UpdateDatabaseDdl or a preceding poll. Inspect/return the cached completed operation outcome before applying the remaining wait budget (the SDK can resolve a completed operation without another RPC). Keep the pending zero-budget handoff and caller-cancellation rules intact. Include terminal errors with cancellation/deadline status codes when checking error classification, so a known operation failure cannot be mistaken for expiration of the local polling context. Prefer simplifying the wait context/error-cause handling over adding another independent timing engine.

Independent existing focused tests passed; an overlay test using newCompletedDDLServer plus opErr and a zero budget fails exactly as above. No tracked files were modified. Please fix this in the existing PR, run the required full make check before push with captured output/exit, and handle CI.

[cursor]

P1 addressed on 744f217facb7bd4b6663d193ce6c8a665b22e561.

Zero-budget ASYNC_WAIT now inspects a cached completed LRO before applying the remaining wait budget. Poll resolves that terminal result without another GetOperation, so a FailedPrecondition (or deadline-coded) completed failure stays a failure instead of a successful async handoff with DONE=true and an empty ERROR cell. Pending zero-budget handoff and caller-cancellation rules are unchanged.

This replies to #974 (comment)

[apstndb]

Independent review of HEAD a3577f7227c3bd60416141525002aca752053e95: no actionable findings remain. Both reported cases are fixed: an in-flight GetOperation is bounded by the ASYNC_WAIT budget, and a cached terminal failure is resolved before zero-budget handoff. Completed operation deadline/cancellation statuses remain terminal failures, while pending-operation local cancellation retains its operation-ID hint.

The independent regression overlay, focused DDL tests and full Docker make check all passed at 744f217facb7bd4b6663d193ce6c8a665b22e561. The current HEAD is an empty CI-trigger commit with exactly the same tree, verified by Git tree IDs. All nine applicable checks on the current HEAD passed; only the conditional Dependabot automerge job is skipped. No unresolved review feedback remains.

[apstndb]

Independent coordinator confirmation on exact HEAD a3577f7227c3bd60416141525002aca752053e95 (base 92f986d6a7ab74566bee82f318b8c44e9f340820).

That SHA is the empty CI retrigger after P1 744f217 (preserve a cached completed LRO failure under a zero ASYNC_WAIT budget). All applicable hosted checks succeeded on the retrigger SHA (test, lint, race, readme-sync, govulncheck, all four cross-compile jobs). Dependabot automerge skipped. MERGEABLE CLEAN. No unresolved review threads. Prior ready JSON on 81c15ff stays withdrawn.

{"repo":"apstndb/spanner-mycli","pr":974,"head":"a3577f7227c3bd60416141525002aca752053e95","base":"92f986d6a7ab74566bee82f318b8c44e9f340820","validation":["hosted test/lint/race/readme-sync/govulncheck/cross-compile success on exact HEAD","MERGEABLE CLEAN","no unresolved review threads"],"deviations":["CLI_ASYNC_DDL removed with no bool alias","did not adopt draft #947"],"blockers":[]}