davsclaus commented on code in PR #27250:
URL: https://github.com/apache/camel/pull/27250#discussion_r4163316786
##########
components/camel-dynamic-router/src/main/java/org/apache/camel/component/dynamicrouter/filter/DynamicRouterFilterService.java:
##########
@@ -170,17 +170,22 @@ public String addFilterForChannel(
* @return the ID of the added filter
*/
public String addFilterForChannel(final PrioritizedFilter filter, final
String channel, final boolean update) {
- boolean filterExists = !filterMap.isEmpty() &&
- filterMap.get(channel).stream().anyMatch(f ->
filter.id().equals(f.id()));
+ Set<PrioritizedFilter> filters = filterMap.computeIfAbsent(channel,
+ c -> new
ConcurrentSkipListSet<>(DynamicRouterConstants.FILTER_COMPARATOR));
+ List<PrioritizedFilterStatistics> filterStatistics =
filterStatisticsMap.computeIfAbsent(channel,
+ c -> Collections.synchronizedList(new ArrayList<>()));
+ boolean filterExists = filters.stream().anyMatch(f ->
filter.id().equals(f.id()));
boolean okToAdd = update == filterExists;
if (okToAdd) {
- Set<PrioritizedFilter> filters = filterMap.computeIfAbsent(channel,
- c -> new
ConcurrentSkipListSet<>(DynamicRouterConstants.FILTER_COMPARATOR));
+ if (filterExists) {
+ // the set is ordered by priority and id: adding the updated
filter would neither replace a filter with
+ // the same priority nor remove the one with the old priority,
so remove the existing filter first
+ // (its statistics stay, as when a filter is removed: they
represent actions that happened)
+ filters.removeIf(f -> filter.id().equals(f.id()));
Review Comment:
Nit (optional, non-blocking): between `removeIf` and `add`, an exchange
routed at the same time can find no filter for this subscription and end up on
the "no filters matched" log endpoint. When the priority changes, you could add
the new filter first and then remove the old instance, which avoids that
window. When the priority is the same, the skip-list set has no atomic replace,
so the short gap is probably fine. Concurrent subscribe/update of the same id
was already racy before this change, so this doesn't need to block the PR.
--
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]
