Replace synchronized multimap with concurrent hash map in NotificationControllerV2 for better performance by nobodyiam · Pull Request #5532 · apolloconfig/apollo

nobodyiam · 2026-01-17T06:50:45Z

What's the purpose of this PR

Replace synchronized multimap with concurrent hash map in NotificationControllerV2 for better performance

Which issue(s) this PR fixes:

Fixes #5450

Brief changelog

Replace synchronized multimap usage in NotificationControllerV2 with a concurrent, case-insensitive multimap implementation to reduce lock contention under high concurrency.
Introduce CaseInsensitiveMultimapWrapper (backed by ConcurrentMap + concurrent Set) to manage deferred results safely and efficiently; normalize keys in a locale-stable way.
Update NotificationControllerV2Test and add CaseInsensitiveMultimapWrapperTest to cover correctness and basic concurrency behavior.

Follow this checklist to help us incorporate your contribution quickly and easily:

Read the Contributing Guide before making this pull request.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit tests to verify the code.
Run mvn clean test to make sure this pull request doesn't break anything.
Run mvn spotless:apply to format your code.
Update the CHANGES log.

Summary by CodeRabbit

Chores
- Improved notification handling with a concurrent, case-insensitive backing to boost throughput and reduce lock contention.
New Features
- Notification keys are now treated case-insensitively, improving matching consistency.
Tests
- Added comprehensive unit tests covering put/get/remove, size, unmodifiable views, custom suppliers, and concurrency scenarios.
Documentation
- Added a performance-focused changelog entry describing the notification handling improvements.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-17T06:50:57Z

📝 Walkthrough

Walkthrough

Replaces a synchronized TreeMultimap in NotificationControllerV2 with a new CaseInsensitiveMultimapWrapper backed by concurrent map/set implementations; adds the wrapper class and comprehensive tests, updates NotificationControllerV2 and its test, and adds a changelog entry describing the concurrency/performance change.

Changes

Cohort / File(s)	Summary
Core Implementation `apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/wrapper/CaseInsensitiveMultimapWrapper.java`	New generic case-insensitive multimap wrapper backed by `Map<String, Set<V>>` with pluggable map/set suppliers. Provides `put`, `remove`, `get`, `containsKey`, and `size`. Normalizes keys with `Locale.ROOT`.
Controller Update `apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/NotificationControllerV2.java`	Replaces `Multimap<String, DeferredResultWrapper>` synchronized TreeMultimap field with `CaseInsensitiveMultimapWrapper<DeferredResultWrapper>` using concurrent map and concurrent-hash-set supplier; imports adjusted.
Tests — New `apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/wrapper/CaseInsensitiveMultimapWrapperTest.java`	New unit tests covering put/get/remove semantics, case-insensitivity, size, unmodifiable get view, custom supplier behavior, and a concurrency race-condition scenario.
Tests — Updated `apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/controller/NotificationControllerV2Test.java`	Adjusted test field type and ReflectionTestUtils casts to `CaseInsensitiveMultimapWrapper<DeferredResultWrapper>`.
Changelog `CHANGES.md`	Added Apollo 2.5.0 entry: "Perf: Replace synchronized multimap with concurrent hashmap in NotificationControllerV2 for better performance".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

size:XL, lgtm

Suggested reviewers

hezhangjian

Poem

🐰
I swapped the locks for hopping haste,
Keys downcased, no time to waste,
Sets race safe, maps run free,
Deferred results now spring with glee,
A little hop for faster spree.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the primary change: replacing synchronized multimap with concurrent data structure in NotificationControllerV2 for performance improvement.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#5450` by replacing synchronized multimap with CaseInsensitiveMultimapWrapper backed by ConcurrentMap, reducing lock contention and improving performance under high concurrency.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the objective: the wrapper class, controller modifications, related test updates, and changelog entry are all necessary for implementing the concurrent multimap replacement.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/wrapper/CaseInsensitiveMultimapWrapperTest.java`:
- Around line 158-167: The test fails because
CaseInsensitiveMultimapWrapper.get(...) returns a Collections.unmodifiableSet,
so asserting set instanceof HashSet is false; change the testCustomSetSupplier
to unwrap the returned unmodifiable set via reflection (the
Collections$UnmodifiableSet has a private field "c" referencing the original
set) and assert that the underlying "c" is a HashSet, or alternatively access
the wrapper's internal map entry for the key and assert that its value is a
HashSet; reference the test method testCustomSetSupplier,
CaseInsensitiveMultimapWrapper.get, and the Collections$UnmodifiableSet "c"
field when making the change.

…nControllerV2 for better performance

dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jan 17, 2026

coderabbitai Bot reviewed Jan 17, 2026

View reviewed changes

Comment thread ...ava/com/ctrip/framework/apollo/configservice/wrapper/CaseInsensitiveMultimapWrapperTest.java

nobodyiam force-pushed the fix-5450 branch from ffda71c to eee2e9f Compare January 17, 2026 07:06

Replace synchronized multimap with concurrent hash map in Notificatio…

0bbb8c7

…nControllerV2 for better performance

nobodyiam force-pushed the fix-5450 branch from eee2e9f to 0bbb8c7 Compare January 17, 2026 07:15

nobodyiam mentioned this pull request Jan 17, 2026

Fix thread blocking in NotificationControllerV2 by replacing synchronized Multimap with case-insensitive concurrent implementation #5451

Closed

5 tasks

nobodyiam merged commit e8384a5 into apolloconfig:master Feb 1, 2026
9 checks passed

nobodyiam deleted the fix-5450 branch February 1, 2026 13:14

github-actions Bot locked and limited conversation to collaborators Feb 1, 2026

nobodyiam added this to the 2.5.0 milestone Feb 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace synchronized multimap with concurrent hash map in NotificationControllerV2 for better performance#5532

Replace synchronized multimap with concurrent hash map in NotificationControllerV2 for better performance#5532
nobodyiam merged 1 commit into
apolloconfig:masterfrom
nobodyiam:fix-5450

nobodyiam commented Jan 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 17, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Review ran into problems

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

nobodyiam commented Jan 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's the purpose of this PR

Which issue(s) this PR fixes:

Brief changelog

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Review ran into problems

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nobodyiam commented Jan 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 17, 2026 •

edited

Loading