matrixorigin / matrixorigin/matrixone

[Bug]: Suspended account keeps existing sessions runnable until the next kill-worker tick

Open
#26,624 0 comments 0 reactions 1 assignee Claimed by @daviszhen View on GitHub
deferred kind/bug
Dominant language
Go
Stars
1.9k
Forks
311
Avg merge
1d 3h
Merged PRs (30d)
768

Description

# [Bug]: Suspended account keeps existing sessions runnable until the next kill-worker tick

## Branch and build

- Branch: `main`
- Commit: `820ee12144439c5b63c03b261f4330ec0364fdb3`
- Fresh isolated cluster: 1 LOG, 1 TN, 2 CN

## Summary

`ALTER ACCOUNT SUSPEND` immediately prevents a new client from
connecting, but already-established sessions on both CNs can still execute SQL
for several seconds. The measured window is bounded by the account connection
kill worker rather than by the successful completion of the `ALTER ACCOUNT`
statement.

This reintroduces the behavior tracked by #15202. Its confirmed expectation
was that no connection may run SQL immediately after an account is suspended.

## Minimal scenario

1. Create a normal account, database, and a one-row table.
2. Establish two sessions through CN1 and one through CN2.
3. Verify the sessions can read the seed row.
4. From the system account, execute `ALTER ACCOUNT SUSPEND`.
5. Immediately and repeatedly execute `SELECT note FROM t WHERE id = 1` on
every already-established session.
6. In parallel, attempt fresh logins through both CNs.

## Actual behavior

New logins are rejected immediately. Existing sessions continue returning the
correct row until their local routine manager runs the next kill-worker cycle,
then are closed. This was reproduced three times on the commit above.

| Run | Existing sessions on CN1 | Existing session on CN2 | Fresh logins |
| --- | --- | --- | --- |
| 1 | 23 successful reads for 2.354 s | 88 successful reads for 8.965 s | rejected immediately on both CNs |
| 2 | 98 successful reads for 10.014 s | 64 successful reads for 6.544 s | rejected immediately on both CNs |
| 3 | 87 successful reads for 8.953 s | 53 successful reads for 5.505 s | rejected immediately on both CNs |

The account metadata already reports `suspend` during each window. Reopening
the account restores fresh login and the table remains unchanged except for
the writes made before suspension.

## Expected behavior

Once `ALTER ACCOUNT SUSPEND` succeeds, no pre-existing session of
that account should be able to execute another SQL statement. Existing
sessions should be cancelled or rejected before the DDL reports success, just
as fresh logins are already rejected.

## White-box observation

`postDropSuspendAccount` enqueues a `KillConn` request on each CN.
`RoutineManager.startKillRoutineWorker` consumes that queue at
`sv.KillRountinesInterval`, whose default is 10 seconds. Therefore the account
status and fresh-login check take effect immediately, while the already-open
session admission path remains usable until the next periodic tick.

Relevant locations:

- `pkg/frontend/authenticate.go` (`postDropSuspendAccount`)
- `pkg/cnservice/server_query.go` (`handleKillConn`)
- `pkg/frontend/routine_manager.go` (`startKillRoutineWorker` and
`KillRoutineConnections`)

## Scope checks

The adjacent account-state paths behave as expected in the same three runs:

- `RESTRICTED` permits existing and new reads but rejects writes on both CNs.
- `OPEN` restores writes for existing and fresh sessions.
- All test accounts are removed after each run.
- Both CNs remain healthy after the scenario.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.