davsclaus commented on code in PR #29202:
URL: https://github.com/apache/camel/pull/29202#discussion_r4480143846


##########
core/camel-support/src/main/java/org/apache/camel/support/cluster/RebalancingCamelClusterService.java:
##########
@@ -215,6 +246,38 @@ protected Integer members() {
         return members != null ? members.size() : 0;
     }
 
+    /**
+     * Whether the view of the partition is held by a route or another user. 
The rebalancing only looks at those views,
+     * so it does not start a view again after its last user has released it.
+     */
+    private boolean isViewInUse(String partition) {
+        if (delegate instanceof AbstractCamelClusterService<?> service) {
+            return service.isViewInUse(partition);
+        }
+        return true;
+    }
+
+    /**
+     * Takes a reference to the view of the partition if it is in use, or 
returns {@code null}. The caller releases it
+     * with {@link #releaseRetainedView(CamelClusterView)}, so the rebalancing 
does not keep a view running after its
+     * last user has released it. A delegate that is not an {@link 
AbstractCamelClusterService} has no such lookup, so
+     * its views are taken with {@code getView} (which may start a released 
view until it is released again).
+     */
+    private CamelPreemptiveClusterView retainViewInUse(String partition) 
throws Exception {
+        if (delegate instanceof AbstractCamelClusterService<?> service) {
+            return (CamelPreemptiveClusterView) 
service.retainViewIfInUse(partition);
+        }
+        return delegate.getView(partition);
+    }
+
+    private void releaseRetainedView(CamelClusterView view) {
+        try {
+            delegate.releaseView(view);
+        } catch (Exception ex) {
+            LOG.warn("Could not release view {}", view.getNamespace(), ex);
+        }

Review Comment:
   Not a leak: `ReferenceCount.release()` (camel-util) decrements the count 
with a CAS before it runs the `onRelease` callback, so a stop that fails here 
does not leave the reference behind. WARN is the right level. Resolving.
   
   _Claude Code on behalf of Claus Ibsen (davsclaus)_



##########
core/camel-support/src/main/java/org/apache/camel/support/cluster/AbstractCamelClusterService.java:
##########
@@ -216,6 +216,35 @@ public void stopView(String namespace) throws Exception {
                 });
     }
 
+    /**
+     * Takes a reference to the view of the namespace only while the view is 
in use (it has references), so the caller
+     * can look at it without creating it or starting it again after its last 
user has released it. The caller must
+     * release the view with {@link #releaseView(CamelClusterView)}.
+     *
+     * @return the view, or {@code null} if it is not in use
+     */
+    CamelClusterView retainViewIfInUse(String namespace) {
+        return LockHelper.supplyWithWriteLock(
+                lock,
+                () -> {
+                    ViewHolder<T> holder = views.get(namespace);
+                    // the view is in use, so this reference does not start it
+                    return holder != null && holder.count.get() > 0 ? 
holder.retain() : null;
+                });

Review Comment:
   Optional nit, fine either way (see the approval). Resolving so it does not 
block the merge.
   
   _Claude Code on behalf of Claus Ibsen (davsclaus)_



-- 
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]

Reply via email to