[fix](brpc) Make secondary package aliases non-owning

[gavinchou]

Proposed changes

Fix DORIS-27128 in the brpc secondary package alias implementation.

The existing brpc secondary_package_name patch registers the secondary service name by copying the primary ServiceProperty, and registers secondary methods by copying the primary MethodProperty as owning entries. During brpc::Server destruction, ClearServices() iterates all service and method map entries:

• copied ServiceProperty with SERVER_OWNS_SERVICE can delete the same service twice
• copied MethodProperty with own_method_status=true can delete the same MethodStatus twice

This matches the cloud MetaService secondary package shutdown/coredump path. The patch now makes secondary aliases non-owning:

• secondary MethodProperty sets own_method_status=false and shares the primary MethodStatus
• secondary ServiceProperty sets ownership=SERVER_DOESNT_OWN_SERVICE and shares the primary service pointer only for lookup

The previous variant regression retry change and the TLS starter change have both been removed from this PR.

Testing

• git diff --check origin/master..HEAD
• Verified the brpc patch with a temporary local apply check: reverse-applied the old patch from origin/master to the local brpc-1.4.0 source copy, then applied the updated patch successfully.

Issue: DORIS-27128

[hello-stephen]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[gavinchou]

/review

[gavinchou]

run buildall

[gavinchou]

run buildall

[gavinchou]

Updated this PR based on the DORIS-27128 TLS/cloud context. The previous variant regression retry change was removed; the PR now fixes OSS BE/cloud starter handling for protocols excluded by tls_excluded_protocols.

[gavinchou]

/review

[github-actions]

Codex automated review failed and did not complete.

Error: Codex completed, but no new pull request review was submitted for the current head SHA.
Workflow run: https://github.com/apache/doris/actions/runs/29601164260

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

[gavinchou]

Corrected again after reviewing the secondary_package shutdown path. The TLS starter change has been removed. This PR now fixes the brpc secondary_package_name patch by making secondary service/method aliases non-owning, avoiding double delete during brpc::Server::ClearServices().

[gavinchou]

run buildall

[gavinchou]

/review

[github-actions]

Codex automated review failed and did not complete.

Error: Codex completed, but no new pull request review was submitted for the current head SHA.
Workflow run: https://github.com/apache/doris/actions/runs/29603170396

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

[github-actions]

Automated review result: request changes

The normal destructor path is improved: when primary and secondary names are distinct, the patch leaves one owner and avoids the duplicate deletes. Three issues still need resolution:

• Explicit RemoveService and registration rollback leave the new aliases in BRPC maps after shared targets are deleted, enabling stale lookup and Start() use-after-free.
• If secondary_package_name equals the primary package, FlatMap assignment overwrites the only owning entries with non-owning copies.
• The Cloud lifecycle test still disables the exact non-empty alias branch, so lookup and owned teardown are untested.

Critical checkpoint conclusions:

• Goal and tests: the direct ClearServices double-free is addressed, but runtime regression coverage and removal/error paths are incomplete.
• Scope and clarity: this is one focused patch; method and service ownership changes are symmetrical.
• Lifecycle and error handling: M-001 is blocking; cleanup must cover explicit removal and every rollback.
• Concurrency: no new race or lock issue was found; BRPC mutation is READY-only and destruction stops/joins before ClearServices.
• Configuration and compatibility: no protocol or storage-format change applies; M-002 is the uncovered configuration edge. No persistence, transaction, FE/BE propagation, data-write, observability, or material performance issue applies.
• Testing: static review only under the runner contract; thirdparty/installed and protoc are absent. Cloud UT does not cover the changed branch because the test clears the configuration.
• User focus: no additional focus was provided; the full PR was reviewed.

Review completion: the single changed file and its BRPC/Cloud upstream and downstream paths were reviewed through three rounds; every candidate was accepted, deduplicated, or dismissed with evidence. The three accepted points are attached inline.

[github-actions]

[P1] Erase these aliases when removing the service

Making the alias non-owning avoids the destructor double free, but BRPC 1.4.0 RemoveMethodsOf erases only md->full_name() (plus the IDL no-namespace alias), and RemoveService erases only sd->full_name()/sd->name(). Therefore AddService with a secondary package followed by RemoveService—or an AddServiceInternal rollback—deletes the primary MethodStatus and, on owning removal paths, the service while these secondary map entries remain. A later Find* returns dangling state, and Start() dereferences the freed status/service. Please track and erase the secondary method and service keys on every removal/rollback before deleting the shared objects.

[github-actions]

[P2] Preserve ownership when the alias equals the primary package

secondary_package_name is unrestricted; for MetaService, setting it to its current package doris.cloud makes secondary_full_name equal sd->full_name() and the derived method keys equal md->full_name(). FlatMap::operator[] then overwrites the just-inserted owning entries with these non-owning copies, so ClearServices/RemoveService no longer deletes the MethodStatus or the SERVER_OWNS_SERVICE allocation. Reject or skip an alias equal to the primary name before assigning it, and preflight any other alias-key collision.

[github-actions]

[P2] Add a runtime regression for this ownership branch

The Cloud lifecycle test currently forces config::secondary_package_name = "" specifically to avoid brpc::Server::~Server() crashing, so no test executes this changed non-owning branch. The reported patch-apply check proves only that the third-party patch applies. Please enable a distinct non-empty alias in a test, assert both service/method names resolve, and destroy or ClearServices() an owning server; the removal and equal-package cases above should also be covered.

[hello-stephen]

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	74.95% (30468/40650)
Line Coverage	59.08% (335706/568183)
Region Coverage	55.81% (281605/504584)
Branch Coverage	57.08% (124887/218780)