github-actions[bot] commented on code in PR #63299:
URL: https://github.com/apache/doris/pull/63299#discussion_r3247539401
##########
be/src/runtime/runtime_query_statistics_mgr.cpp:
##########
@@ -230,8 +219,7 @@ TReportExecStatusParams
RuntimeQueryStatisticsMgr::create_report_exec_status_par
TReportExecStatusParams req;
THRIFT_MOVE_VALUES(req, query_profile, profile);
req.__set_backend_id(ExecEnv::GetInstance()->cluster_info()->backend_id);
- // invalid query id to avoid API compatibility during upgrade
- req.__set_query_id(TUniqueId());
+ req.__set_query_id(query_id);
Review Comment:
This breaks the mixed-version profile-reporting contract. Before this change
the profile-only RPC deliberately sent an invalid `query_id` (the removed
comment says it was to avoid API compatibility issues during upgrade), so an
older FE would not route this profile-only request into normal
`updateFragmentExecStatus`. Now a new BE sends the real query id while omitting
`status`, and an old FE that does not have the new early return can find the
coordinator and process a status-less exec report, while also rejecting
`query_profile` because it only understands `fragment_id_to_profile`. Please
keep the old compatibility behavior or add a version-compatible path before
changing this field.
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java:
##########
@@ -224,48 +225,56 @@ public Status updateProfile(TQueryProfile profile,
TNetworkAddress backendHBAddr
return new Status(TStatusCode.INVALID_ARGUMENT, "QueryId is not
set");
}
- if (!profile.isSetFragmentIdToProfile()) {
- LOG.warn("{} FragmentIdToProfile is not set",
DebugUtil.printId(profile.getQueryId()));
- return new Status(TStatusCode.INVALID_ARGUMENT,
"FragmentIdToProfile is not set");
+ if (!profile.isSetFragmentIdToProfileNodeReports()) {
Review Comment:
This makes a new FE reject profile reports from old BEs during a rolling
upgrade. The thrift struct still has the legacy `fragment_id_to_profile` field,
and old BEs will continue to send only that field until they are upgraded; with
this guard, `updateProfile()` returns `INVALID_ARGUMENT` and drops those
profiles instead of translating the old `TDetailedReportParams` format. Please
keep a fallback for `fragment_id_to_profile` until mixed-version reporting is
no longer supported.
--
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]
