morningman opened a new pull request, #68236:
URL: https://github.com/apache/doris/pull/68236
Cherry-picked from #67820, #68088
Batch pick of every merged PR carrying the `incremental-computation` label
that `branch-incremental-computation` does not have yet (no
`incremental-computation-picked` label), in the order they landed on master
(`git log --first-parent`). One commit per PR, each ending with `(cherry picked
from commit <master sha>)`. Follows the same convention as #67830, #68017,
#68073 and #68151.
| # | Master commit | PR | Title |
|---|---|---|---|
| 1 | 44e3ae2b951 | #67820 | [fix](binlog) Track committed TSO and fence
uncertain commits for bounded incremental reads |
| 2 | 3de3a756f74 | #68088 | [fix](binlog) Require SELECT privilege for
binlog TVF |
Not included on purpose:
- The 25 labelled PRs that already carry `incremental-computation-picked`.
- #68012 carries the label but is a PR against this branch itself (merged as
`6f7c87fa892`); nothing to pick.
### How #67820 was picked
This branch already carried #67820 through #67861, which backported the PR's
first seven commits (up to `17272039558`) before the PR was merged. Between
that cut and the merge the PR gained six more commits and five master merges,
and the final design differs from what #67861 brought: the Meta Service
recovery RPC (`get_tso_recovery_transactions`) and the FE startup/periodic
recovery scan are gone, replaced by a durable per-instance commit-TSO fence
(`txn_tso_fence_key`, `advance_tso_fence`, commit-time `TXN_COMMIT_TSO_EXPIRED`
check, `enable_check_commit_tso_fence`), plus the review-feedback and "release
maybe-committed TSO" fixes.
A plain `cherry-pick -x 44e3ae2b951` conflicts in 23 files because the
branch holds the intermediate design, so commit 1 was built by replaying, on
top of the branch, exactly what the PR gained after the cut:
- the PR's later commits `d16e67197b2`, `e53291d196b`, `864546d5531`,
`8c55eb086e9`, `8d264ce294a`, `fc4f9969202`;
- the content that landed inside its master merges, identified by diffing
each merge against its `git merge-tree` automerge result: `c491a293426`
(`setEnvTSOService` replaced by
`Mockito.doReturn(tsoService).when(masterEnv).getTSOService()` — `masterEnv` is
the same Mockito delegating mock here since #67813) and `29f133584b5` (the
extra `TSOTransactionTrackerTest` coverage and two comment removals in
`DorisFlightSqlProducer`). The other three merges only resolved import blocks
against master-only code.
The replay then got squashed into one commit with the master PR's message,
the original author, and the `(cherry picked from commit
44e3ae2b9518e1f11595259136ecd3e5d2d3555a)` trailer. Adaptations, all recorded
in the commit message:
- `DorisFlightSqlProducer` / `DorisFlightSqlProducerTest` stay under
`service/arrowflight` (the branch lacks the package move of #67866).
- `8d264ce294a` is a no-op here: it removes a `catch
(FlightRuntimeException)` block that #67883 added on master and this branch
never had.
- The branch-only `get_tso_recovery_transactions` RPC, its recovery scan,
the five `TsoRecovery*` Meta Service tests and the NOLINT suppressions #67861
had added for them are removed, as on master.
- The round-3 `mockVersionHelper()` adaptation in
`CloudGlobalTransactionMgrTest` (no `VersionHelper.getVersionFromMeta(req,
maxAttempts)` overload here, #66296) is kept.
### Prerequisite check
- **#67820** declares #67181 and #67594 as related; #67181 (`e5a4e725fac`)
is before the fork point and #67594 came with #67830. The rest of what the
commit touches on master is import-block and neighbouring-code drift from
unlabelled commits (#67866 / #67883 / #67966 Arrow Flight and session
refactors, #67761 `get_prepare_txn_by_coordinator`, #66598 pre-rowset delete
bitmaps, repair-tablet-index changes); none of it is used by the
fence/committed-TSO logic.
- **#68088** declares no related PR. The hook it implements
(`TableValuedFunctionIf.checkAuth`) and the caller chain
(`CheckPrivileges.visitLogicalTVFRelation` → `TableValuedFunction.checkAuth` →
catalog function) are byte-identical between this branch and master, and the
five-argument `AccessControllerManager.checkTblPriv` overload exists. Applied
cleanly.
### Drift check against master
- **#67820**: every one of the 54 files the master commit touches now
contains the pick's content — the master commit reverse-applies cleanly per
file onto this branch (50 files), and the four files where only the surrounding
context differs (`config.h`, `meta_service_txn.cpp`, `StmtExecutor.java`,
`StmtExecutorTest.java`) contain every added line and none of the removed ones.
33 of the 54 files are byte-identical to master at `44e3ae2b951`, including all
of `fe/.../tso/`, `CloudGlobalTransactionMgr.java`, `MetaServiceProxy.java`,
`keys.{h,cpp}`, `meta_service.h` and the regression suite/output. The remaining
differences are unrelated master-only or branch-only code from unlabelled
commits (#67761, #66598, #67866/#67883/#67966, repair-tablet-index, meta-cache
columns, recycler configs) plus the documented `mockVersionHelper()`
adaptation; no line in the committed-TSO/fence domain is left over from #67861.
- **#68088**: all three files are byte-identical to master at `3de3a756f74`.
### Verification
- FE: `run-fe-ut.sh --run` on this branch (regenerates thrift/protobuf,
compiles fe-core main + test) with every test class the picks touch: 12
classes, 175 tests, 0 failures, 0 errors, BUILD SUCCESS — `TSOServiceTest` 38,
`CloudGlobalTransactionMgrTest` 37, `StmtExecutorTest` 27,
`MetaServiceProxyTest` 22, `OlapScanNodeTest` 12,
`TimeBasedChangeVisibleWaiterTest` 12, `TSOTransactionTrackerTest` 9 (the four
tests added inside the PR's last master merge included),
`DorisFlightSqlProducerTest` 7, `TsoStatusMetadataGeneratorTest` 5,
`CloudCommittedTsoTest` 3, `SchemaTableTest` 2, `TableBinlogFunctionAuthTest` 1
(#68088). The `@Test` counts of the touched classes equal master's.
- FE checkstyle on fe-core: 0 violations.
- Meta Service: `-fsyntax-only` with the flags of `cloud/CMakeLists.txt`
(`-Wall -Werror`, regenerated `gen_cpp/cloud.pb.h` with `AdvanceTsoFence*` /
`TxnTsoFencePB` and without `GetTsoRecoveryTransactions*`) on
`meta_service_txn.cpp`, `keys.cpp`, `http_encode_key.cpp`, `bvars.cpp` and,
with `-DUNIT_TEST -DBE_TEST -fno-access-control`, on `meta_service_test.cpp`,
`keys_test.cpp`, `http_encode_key_test.cpp`, `meta_service_helper_test.cpp`,
`txn_lazy_commit_test.cpp`: no diagnostic in any line the pick touches (the
only errors are the pre-existing macOS-only `pthread_setname_np` /
`int64_t`-vs-`long` ones in untouched 2024/2025 code). clang-format 16 is clean
on all 14 touched cloud files.
- No BE file changes in this round (the BE side of #67820 was already
byte-identical to master via #67861).
- The new `auth_p0/test_binlog_tvf_auth.groovy` and the two #67820 suites
parse cleanly (groovy parser check).
--
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]
