morningman opened a new pull request, #70393:
URL: https://github.com/apache/doris/pull/70393
### Background
When a frontend becomes master, its `InternalSchemaInitializer` daemon
thread brings `__internal_schema.audit_log` to the audit schema of its version
(`InternalSchema.AUDIT_SCHEMA`), retrying every 5 seconds until that succeeds.
A schema change that only adds value columns is light: the FE updates the
metadata and nothing is rewritten. Reordering columns turns it into a job that
rewrites every tablet on a backend, and in cloud mode that backend comes from
the compute group of the current session. Audit events are loaded by column
name, so the column order does not matter for loading.
### What problem does this PR solve?
Issue Number: None
Related PR: #51363 (introduced the add + reorder upgrade), #52432 (its pick
to branch-3.1)
PR Type: Bugfix
Problem Summary:
In cloud mode, a cluster upgraded to a release that adds audit columns never
gets them: after 4.1.3 -> 4.1.5, for example, `audit_log` has no
`queue_time_ms`, and the master FE logs `Failed to alter audit table schema ...
CONNECT_CONTEXT_NOT_SET` every 5 seconds for as long as it runs, restarts
included. The upgrade was always an add + reorder job, which in cloud mode
needs a compute group, and the initializer's thread has no session to resolve
one. This PR adds missing columns in place by a light schema change when the
table's columns are already in order, as in every table created since 3.1, and
runs the ALTER in a session of its own, so the rewrite a 3.0 table still needs
also works in cloud mode. Most of the diff is tests.
### Details
- **Change map:** +60 -18 lines of production code, all in
`InternalSchemaInitializer`, and +371 -128 of tests (86% of the added lines).
The code has two parts: (1) `getAuditTableAlterOps` plans the ALTER: an `ADD
COLUMN ... AFTER` op per missing column when the table's columns are in order,
the existing add + reorder otherwise (about 50 lines); (2)
`alterAuditSchemaIfNeeded` runs the ALTER inside
`StatisticsUtil.buildConnectContext(false)` (about 10 lines). The tests:
planner cases in `InternalSchemaInitializerTest`, both paths on an FE in
`InternalSchemaAlterTest`, and a new docker suite that restarts the FE on an
older table.
- **Frequency:** Always, in cloud mode, when the master FE finds `audit_log`
short of columns of its schema: after 3.0 -> 3.1/4.0, 4.0 or 4.1.0-4.1.3 ->
4.1.4+/4.2 (`queue_time_ms`), and 4.1/4.2 -> master (`protocol` and two spill
columns). Fresh clusters are not affected.
- **Root cause:** Since #51363, `alterAuditSchemaIfNeeded` always adds a
`ReorderColumnsOp`, so the change is a rewrite job.
`SchemaChangeHandler.createJob` asks each replica for its backend; in cloud
mode `CloudReplica.getBackendId()` ->
`CloudSystemInfoService.getCurrentClusterId()` throws `CONNECT_CONTEXT_NOT_SET`
without a `ConnectContext`, which the initializer's thread never has. The
exception is logged and the loop retries forever.
- **Why existing tests missed it:** Unit tests: `InternalSchemaAlterTest`
creates the tables fresh, so the upgrade has nothing to do; the two audit tests
of `InternalSchemaInitializerTest` re-implemented the old logic inside the test
and never called production code; nothing ran the initializer in cloud mode.
Regression: `audit/test_audit_log_queue_time`, `audit/test_audit_log_protocol`
and the cloud `audit/test_audit_log_hint_compute_group_docker` all start on a
fresh audit table; none restarts the FE on an older one.
- **Fix:** Missing columns follow their predecessor in the schema, in one
ALTER, when the schema's columns the table has are in order and come first: a
light change that needs no job, backend or compute group. Anything else (a 3.0
table, an unknown column in between) keeps add + reorder, now in a session with
ADMIN and, in cloud mode, a compute group, as `createTable()` of the class
already does. The fix belongs here: only this daemon thread created a rewrite
job without a session.
- **Other paths and branches:** `modifyColumnStatsTblSchema()` in the same
thread also alters without a session, but its ops are light on every table a
cloud-capable version created, so it is unchanged; `modifyTblReplicaCount`
skips cloud mode. branch-3.1 (#52432), branch-4.0, branch-4.1 and branch-4.2
all have the bug.
- **Behavior change:** Shared-nothing clusters, where the upgrade worked, no
longer rewrite an audit table that only lacks columns: `SHOW ALTER TABLE
COLUMN` shows a finished light change (`IndexId` = `OriginIndexId`,
`TransactionId` -1) instead of a rewrite job. In cloud mode, a table that needs
reordering is rewritten on the compute group resolved for ADMIN, which by the
code can start a suspended one.
### Testing
- FE unit tests, run locally with `run-fe-ut.sh` on this head:
`InternalSchemaInitializerTest` (8, 5 new, including a 4.1.3 table upgraded in
place with the schema's types and a 3.0 table reordered),
`InternalSchemaAlterTest` (5, 2 new, run like the daemon on a thread without a
session), `AlterJobV2QueryGlobalsTest` (1). Before the fix the two new
`InternalSchemaAlterTest` cases fail: `expected: <NORMAL> but was:
<SCHEMA_CHANGE>` and `expected: <'admin'@'%'> but was: <null>`.
- New docker suite `audit/test_audit_log_schema_upgrade_docker`: drops three
audit columns and restarts the FE, expecting them back in place with the same
tablets and filled by new audit rows; then moves `catalog` to the end,
restarts, and expects the rewrite job to finish. Run locally before the rebase
onto #63612, which only adds cache invalidation to these alter paths, with an
FE of this branch and BE/MS 4.1.3 (no master BE image for macOS; that FE build
turned two new planner features off and set `be_exec_version = 10`, not part of
this PR):
| Mode | Before the fix | This PR |
|---|---|---|
| cloud | fails: `the audit table was not upgraded`; `Failed to alter
audit table schema ... CONNECT_CONTEXT_NOT_SET` every 5 s | passes, no such
warning |
| shared-nothing | fails: the upgrade rewrote the table, all 8 tablets
replaced | passes; the in-place change also replays on restart |
- No pipeline config in this repository sets `excludeDockerTest = false`, so
whether CI runs the suite depends on TeamCity, which I could not reach to check.
### Notes for Reviewers
- `alterAuditSchemaIfNeeded` became `@VisibleForTesting public static`, like
`createTbl` of the class, so a test can run it as the daemon does.
- The in-place ops build columns through
`ColumnDef.translateToColumnDefinition()`, as the class's stats-table code
does, because `AddColumnOp` needs a `ColumnDefinition` for its SQL text; the
tests check the resulting types against the schema.
- When picking to a release branch, `testAuditTableOf413AddedInPlace`
expects master-only columns and needs adjusting.
### Release note
Fix `__internal_schema.audit_log` never getting the columns of a newer
version after an upgrade in cloud mode, while the master FE logged `Failed to
alter audit table schema ... CONNECT_CONTEXT_NOT_SET` every few seconds. New
audit columns are now added in place, without rewriting the table.
### Check List (For Author)
- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Ready
### AI Disclosure
- Tool: Claude Code (Claude Opus 5.5)
- Generated: the fix, the unit tests, the docker regression suite, and this
description
- Human review: not stated
- Prompts: Asked to explain a user report of the audit table upgrade failing
in cloud mode, whether an FE restart recovers, and why the upgrade uses a heavy
schema change; then to fix it on a new branch, preferring a light schema change
without needless reordering, wrapping the ALTER in
`StatisticsUtil.buildConnectContext(false)` as `createTable()` does as the
fallback, and adding unit and end-to-end tests.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
