morningman opened a new pull request, #70121:
URL: https://github.com/apache/doris/pull/70121
### What problem does this PR solve?
Issue Number: None
Related PR: #70118
Problem Summary:
**In short.** When an FE rebuilds a routine load job from metadata (a
follower replaying the edit log, a restart, a new master), the job loses
`max_filter_ratio`, `exec_mem_limit`, `send_batch_parallelism`,
`load_to_single_tablet`, `enclose`, `escape` and the
`enable_memtable_on_sink_node` it was created with, and runs with defaults from
then on. Separately, `ALTER ROUTINE LOAD` on `enclose` / `escape` never reaches
the running job. This PR restores them when the job is rebuilt and applies the
altered values.
**Background.** A routine load job is persisted with GSON, which writes only
the fields annotated with `@SerializedName`; these seven are plain fields.
`gsonPostProcess` re-parses the stored CREATE statement, but only to rebuild
the `RoutineLoadDesc` (columns, filters, separators, partitions, merge type),
and restores the partial update settings from the persisted `jobProperties`
map. CREATE records the other values in `jobProperties` as well (all but
`exec_mem_limit`), and SHOW ROUTINE LOAD prints them from there, so the loss
does not show. Found while making `SHOW CREATE ROUTINE LOAD` print these
properties (#70118).
**The problem, and what it cost.** Measured on master `856194ece0e` with a
unit test that creates jobs from SQL and rebuilds them with a GSON round trip
on a thread without a session, as a follower or a restarting FE does:
| | Job as created | Job rebuilt from metadata |
|---|---|---|
| `max_filter_ratio` = 0.5 | 0.5 | 1.0: up to every row may be filtered
instead of the job pausing |
| `exec_mem_limit` = 4294967296 | 4 GB | 2 GB |
| `send_batch_parallelism` = 3, or 4 from the creating session | 3 / 4 | 1 |
| `load_to_single_tablet` = true | true | false |
| `enclose` `"`, `escape` `\` | `"` / `\` | none: quoted CSV fields are
split at the separators inside the quotes |
| `enable_memtable_on_sink_node` of the creating session (on by default) |
true | false |
| `ALTER ... ("enclose" = "'", "escape" = "\|")` | tasks keep `"` / `\` |
SHOW ROUTINE LOAD shows the new values, tasks never use them |
| `ALTER ... ("enclose" = "ab")` | accepted, without effect | CREATE rejects
it |
The CSV and filter ratio cases change what gets loaded, without any error,
after an event (a failover, a restart) that has nothing to do with the job.
**How this PR fixes it.**
- `gsonPostProcess` restores `max_filter_ratio`, `send_batch_parallelism`,
`load_to_single_tablet`, `enclose` and `escape` from `jobProperties`, in the
loop that already restores the partial update settings from there. CREATE has
recorded these values since each property was introduced, and ALTER records the
new ones; a job created before one of them existed has no key and keeps the
default, as today. `exec_mem_limit`, which no ALTER changes and `jobProperties`
does not hold, is taken from the CREATE statement that `gsonPostProcess`
already re-parses.
- CREATE records the creating session's `enable_memtable_on_sink_node` in
the job's persisted session variables, next to `sql_mode`, and
`gsonPostProcess` restores it. A job created before this has no record and
keeps `false`, which is what it has run with after every restart so far.
- `ALTER ROUTINE LOAD` checks `enclose` and `escape` the way CREATE does,
and `modifyCommonJobProperties`, which Kafka and Kinesis jobs and the replay of
an ALTER share, installs them on the job. The two rules move into
`CsvFileFormatProperties.parseEnclose` / `parseEscape`, which
`analyzeFileFormatProperties` now calls without any change in behavior.
- A value that an older ALTER stored without the check is ignored with a
warning, as it always was, instead of failing the replay of the job (which
would cancel it) or of the ALTER.
No new persisted field: the only addition is one key in the job's existing
session variable map, which older FEs ignore.
**Results.**
| | Before | After |
|---|---|---|
| declared properties above, job rebuilt from metadata | defaults | as
declared, also in what tasks are built from |
| `send_batch_parallelism` and `enable_memtable_on_sink_node` from the
creating session | 1 / false | the session's values |
| ALTER of `enclose` / `escape` | shown, never used | used by the next task,
kept when the job is rebuilt |
| ALTER with `"enclose" = "ab"` | accepted | `enclose should not be longer
than one byte.` |
| such a value stored by an older ALTER | no effect | no effect, the replay
goes on |
branch-4.1 and branch-4.2 have the same gaps.
### Release note
Routine load jobs keep max_filter_ratio, exec_mem_limit,
send_batch_parallelism, load_to_single_tablet, enclose, escape and the
enable_memtable_on_sink_node of the creating session on followers and after an
FE restart or a master switch, instead of falling back to the defaults. ALTER
ROUTINE LOAD now applies a new enclose or escape to the running job, and
rejects the values CREATE ROUTINE LOAD rejects.
### 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? -->
New `RoutineLoadJobReplayTest` (4 cases on a mocked FE) rebuilds jobs
with a GSON round trip on a thread without a session: the declared properties
survive, including in what tasks are built from; the values taken from the
creating session survive; an altered enclose and escape reach the tasks and
survive a rebuild; an invalid value from an older ALTER neither applies nor
fails the replay. `AlterRoutineLoadCommandTest` gets a case for the new check.
15 test classes (routine load, ALTER, CSV and JSON file formats, load
commands), 146 cases, pass. With the production files reverted to master, all 5
new cases fail, and each of the 12 parts of the fix, removed on its own, fails
its assertion. Kinesis jobs share the changed code paths but need an AWS
endpoint to create, so they have no case of their own.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
A job rebuilt from metadata keeps these values instead of the
defaults. ALTER ROUTINE LOAD rejects an invalid enclose or escape, which it
used to accept without effect, and applies a valid one.
- 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
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
