gnodet-bot commented on code in PR #29202:
URL: https://github.com/apache/camel/pull/29202#discussion_r4475109472
##########
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:
💡 **Nit (non-blocking):** The ternary combines a null guard, a count check,
and the retain call on one line. A small refactor improves readability:
```suggestion
if (holder == null || holder.count.get() == 0) {
return null;
}
return holder.retain();
```
##########
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:
💡 **Suggestion:** A failed `releaseView` leaves the `ViewHolder` refcount
not decremented — the view will keep running indefinitely. This deserves
`LOG.error` to ensure operators notice the ref-count leak:
```suggestion
LOG.error("Could not release view {}", view.getNamespace(), ex);
```
--
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]
