github-actions[bot] commented on code in PR #66918:
URL: https://github.com/apache/doris/pull/66918#discussion_r3819504931
##########
be/src/service/internal_service.cpp:
##########
@@ -1985,22 +1985,13 @@ void
PInternalServiceImpl::response_slave_tablet_pull_rowset(
void PInternalService::multiget_data(google::protobuf::RpcController*
controller,
const PMultiGetRequest* request,
PMultiGetResponse* response,
google::protobuf::Closure* done) {
- bool ret = _heavy_work_pool.try_offer([request, response, done]() {
- signal::SignalTaskIdKeeper keeper(request->query_id());
- // multi get data by rowid
- MonotonicStopWatch watch;
- watch.start();
- brpc::ClosureGuard closure_guard(done);
- response->mutable_status()->set_status_code(0);
-
SCOPED_ATTACH_TASK(ExecEnv::GetInstance()->rowid_storage_reader_tracker());
- Status st = RowIdStorageReader::read_by_rowids(*request, response);
- st.to_protobuf(response->mutable_status());
- LOG(INFO) << "multiget_data finished, cost(us):" <<
watch.elapsed_time() / 1000;
- });
- if (!ret) {
- offer_failed(response, done, _heavy_work_pool);
- return;
- }
+ brpc::ClosureGuard closure_guard(done);
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+ // The deprecated response field is retained only to reject legacy callers
explicitly.
+ Status::NotSupported("multiget_data is deprecated; use multiget_data_v2")
Review Comment:
[P1] Preserve the legacy path through the BE-first upgrade window
Doris's [documented rolling-upgrade
flow](https://doris.apache.org/docs/4.x/admin-manual/cluster-management/upgrade/)
upgrades BEs before FEs, but the released 4.1.3 FE can still produce this
request. With `topn_lazy_materialization_threshold=-1` and the default
`enable_two_phase_read_opt=true`, its `DeferMaterializeTopNResult` injects
`__DORIS_ROWID_COL__` and the translator installs `TFetchOption` for an
eligible small TopN. The old fetcher sends `multiget_data` to every
query-available BE, so this `NotSupported` response from even one upgraded BE
aborts the query; if phase 1 itself lands on an upgraded BE, the removed row-ID
iterator/result-sink consumers break that plan too. Please retain the complete
legacy BE path for the supported rolling-compatibility window and cover
old-FE/new-BE execution in a mixed-version test. A negotiated
minimum-FE/capability gate would be equivalent only if it is first backported,
released, and made a prerequisite for every supported old FE.
--
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]
