morningman opened a new pull request, #70125:
URL: https://github.com/apache/doris/pull/70125
### What problem does this PR solve?
Issue Number: None
Related PR: #69749, #62606
Problem Summary:
**In short.** In cloud mode, `REFRESH MATERIALIZED VIEW ... INCREMENTAL WITH
DRY RUN` and `EXPLAIN REFRESH MATERIALIZED VIEW` of an MV that declares no
`compute_group` plan (and the dry run executes) in whatever compute group the
`admin` user resolves to, not in the group of the session that issues them,
where a manual `REFRESH` of the same MV runs. This PR binds both to the
session's group, the way a manual REFRESH borrows it.
**Background.** Both statements build an internal context with the admin
identity and the MV's session variables (`MTMVPlanUtil.createMTMVContext`). Its
`cloud_cluster` is empty, so it resolves a group the way admin would: admin's
`default_compute_group`, else the first group admin may use. A manual REFRESH
passes the session's group to its task instead (`MTMVJobManager#refreshMTMV`),
and the task binds it when the MV declares none (`MTMVTask#applyComputeGroup`).
#69749 made both statements honour a declared group; an MV that declares none,
which is every MV created before #69749 and any MV created without the
property, still goes to admin's group. Found while reviewing #69749 (thread
4401555648).
**The problem, and what it cost.** Measured on master `856194ece0e` with a
unit test in cloud mode: groups `cg_of_admin` and `cg_of_session`, the session
on `USE @cg_of_session`, an MV that declares no group.
| | manual REFRESH | dry run | EXPLAIN REFRESH |
|---|---|---|---|
| compute group | `cg_of_session` | `cg_of_admin` | `cg_of_admin` |
- If the MV's `workload_group` exists only in the session's group (a
workload group belongs to one compute group in cloud mode), the dry run fails
with `Can not find workload group <wg> in compute croup cg_of_admin`, while the
manual REFRESH succeeds.
- Otherwise the dry run's delta query runs on the backends of another group,
which the session's user may have no USAGE on, and starts that group if it is
suspended. EXPLAIN REFRESH assigns backends there and can start it as well.
Results are not affected: the dry run only reads, and storage is shared.
**How this PR fixes it.**
- The private lookup of the session's group in `MTMVJobManager` moves
unchanged to `MTMVPlanUtil.getSessionComputeGroup()`, so a manual REFRESH, a
dry run and EXPLAIN REFRESH resolve it with the same code.
- `RefreshMTMVCommand` builds both internal contexts through one helper. It
reads the session's group first, while the session is still the thread-local
context, then binds the declared group, else the session's group, as
`MTMVTask#applyComputeGroup` does. With neither, the context stays unbound and
the existing resolution applies; outside cloud mode nothing changes.
- As in `MTMVTask`, the dry run re-checks only a declared group, since the
MV never chose the session's group. EXPLAIN REFRESH executes nothing and still
checks nothing.
**Results.**
| MV | session | dry run / EXPLAIN REFRESH, before | after |
|---|---|---|---|
| declares no group | `USE @cg_of_session` | `cg_of_admin` |
`cg_of_session`, as a manual REFRESH |
| declares no group | resolves no group | admin's resolution | unchanged |
| declares `cg_declared` | `USE @cg_of_session` | `cg_declared` | unchanged |
| any, outside cloud mode | | no compute group | unchanged |
branch-4.1 and branch-4.2 have neither statement.
### Release note
In cloud mode, `REFRESH MATERIALIZED VIEW ... WITH DRY RUN` and `EXPLAIN
REFRESH MATERIALIZED VIEW` of a materialized view that declares no
`compute_group` run in the compute group of the current session, as a manual
`REFRESH` does, instead of the group the admin user resolves to.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
`RefreshMTMVCommandTest` (mocked cloud mode) gets 4 cases: without a
declaration, the dry run and the EXPLAIN contexts resolve to the session's
group; a declared group still wins over it; the dry run does not re-check a
borrowed group; and an existing case now runs from a session that resolves no
group, where both contexts stay unbound. On master the first two fail
(`expected: <cg_of_session> but was: <cg_of_admin>`). Together with the classes
that call the changed code (`ExplainRefreshMTMVCommandTest`,
`MTMVJobManagerTest`, `MTMVPlanUtilTest`, `MTMVTaskTest`,
`MTMVComputeGroupTest`, `IvmIncrRefreshMTMVTest`, `IvmBaselineRebuildTest`,
`ProtocolCapabilityWiringTest`, `NereidsParserTest`), 10 classes and 335 cases
pass. Each part of the fix, broken on its own, fails its case: reading the
session's group after the internal context is installed, letting the session's
group win over a declared one, re-checking the borrowed group, and failing when
the session resolves no group. Not
run on a cloud cluster: the dry run needs IVM and changelog support in the
backends, so a local cloud cluster needs a master backend image; group
selection and the workload group lookup both happen in the FE, which the cases
exercise with the real code.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
In cloud mode, the dry run and EXPLAIN REFRESH of an MV that declares
no compute group use the session's compute group instead of the one admin
resolves to.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
--
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]
