github-actions[bot] commented on code in PR #64762:
URL: https://github.com/apache/doris/pull/64762#discussion_r3472758815
##########
be/src/service/internal_service.cpp:
##########
@@ -1346,16 +1346,19 @@ void
PInternalService::report_stream_load_status(google::protobuf::RpcController
const
PReportStreamLoadStatusRequest* request,
PReportStreamLoadStatusResponse* response,
google::protobuf::Closure*
done) {
+ brpc::ClosureGuard closure_guard(done);
TUniqueId load_id;
load_id.__set_hi(request->load_id().hi());
load_id.__set_lo(request->load_id().lo());
- Status st = Status::OK();
+ Status st = request->has_status() ? Status::create(request->status()) :
Status::OK();
Review Comment:
This behavior still needs a focused test in the final PR. The final file
list only changes `internal_service.cpp`, and I do not see any remaining test
for `PReportStreamLoadStatusRequest` / `report_stream_load_status`; the unit
test that was added in the first commit was deleted again in the final commit,
while the PR body still claims it was run. This handler now has several
observable contracts: the closure must run, an unknown load id must return a
non-OK RPC response without dereferencing a null context, a non-OK request
status must reach `StreamLoadContext::load_status_promise`, and a successfully
delivered report should still acknowledge the RPC with OK. Please restore/add a
BE unit test covering those cases so this regression fix is actually locked
down.
--
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]
