Skip to content

fix(registry): restore invoker URL de-duplication and keep #16108 protocol-matching fix#16133

Open
onceMisery wants to merge 4 commits intoapache:3.2from
onceMisery:fix/issues/16108
Open

fix(registry): restore invoker URL de-duplication and keep #16108 protocol-matching fix#16133
onceMisery wants to merge 4 commits intoapache:3.2from
onceMisery:fix/issues/16108

Conversation

@onceMisery
Copy link

What is the purpose of the change?

This PR fixes a regression introduced while addressing #16108.

Background

During the #16108 fix, URL de-duplication in ServiceDiscoveryRegistryDirectory#refreshInvoker() was accidentally removed.
If registry sends duplicated instance URLs in one notification batch, toInvokers() may call protocol.refer() multiple times for the same endpoint before map finalization. The later put() overwrites earlier entries in newUrlInvokerMap, which can leave previously created invokers untracked and not destroyed.

This regresses the behavior fixed by commit a35af2742a (Remove duplicated invokers in registry notification).

What is changed

  1. Restore invoker URL de-duplication in refreshInvoker()
  • Re-introduce invokerUrls.stream().distinct() before toInvokers().
  • Keep duplicate-notification log with original vs distinct size.
  1. Keep [Bug] SpringCloudServiceInstanceNotificationCustomizer causes "Unsupported protocol" error when Spring Cloud and Dubbo services coexist on the same Nacos namespace #16108 logic correction
  • Spring Cloud instance matching now requires consumer protocol to be explicitly rest.
  • Avoid matching Spring Cloud instances when consumer protocol is null or non-REST.
  1. Add regression test coverage
  • SpringCloudServiceInstanceNotificationCustomizerTest verifies protocol-match behavior and mixed-instance handling.
  • ServiceDiscoveryRegistryDirectoryTest verifies duplicated URLs in one notify batch are de-duplicated and do not trigger repeated refer flow.

Why this is needed

GitHub Issue

Checklist

  • Make sure there is a GitHub_issue field for the change.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.23%. Comparing base (492f78e) to head (d0a7a16).

Files with missing lines Patch % Lines
...stry/client/ServiceDiscoveryRegistryDirectory.java 0.00% 11 Missing ⚠️
...ingCloudServiceInstanceNotificationCustomizer.java 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (492f78e) and HEAD (d0a7a16). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (492f78e) HEAD (d0a7a16)
integration-tests 1 0
Additional details and impacted files
@@             Coverage Diff              @@
##                3.2   #16133      +/-   ##
============================================
- Coverage     41.27%   36.23%   -5.04%     
+ Complexity    10492     9180    -1312     
============================================
  Files          1652     1652              
  Lines         71492    71497       +5     
  Branches      10175    10178       +3     
============================================
- Hits          29508    25907    -3601     
- Misses        37847    41658    +3811     
+ Partials       4137     3932     -205     
Flag Coverage Δ
integration-tests ?
samples-tests 36.23% <0.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants