davsclaus commented on code in PR #27906:
URL: https://github.com/apache/camel/pull/27906#discussion_r4284825361
##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java:
##########
@@ -281,29 +283,17 @@ public List<PendingValue> putSubscriptions(final
Map<UInteger, Subscription> sub
}
}
- if (!this.badSubscriptions.isEmpty()) {
- SubscriptionManager.this.executor.schedule(this::resubscribe,
SubscriptionManager.this.reconnectTimeout,
- TimeUnit.MILLISECONDS);
- }
-
return pendingValues;
}
- private void handleSubscriptionError(
- final StatusCode statusCode, final UInteger clientHandle,
final Subscription s,
- final List<PendingValue> pendingValues) {
- this.badSubscriptions.put(clientHandle, s);
- pendingValues.add(new PendingValue(s.getValueConsumer(), new
DataValue(statusCode)));
- }
-
- private void resubscribe() {
- final Map<UInteger, Subscription> subscriptions = new
HashMap<>(this.badSubscriptions);
- this.badSubscriptions.clear();
- try {
- putSubscriptions(subscriptions).forEach(PendingValue::deliver);
- } catch (final Exception e) {
- handleConnectionFailure(e);
- }
+ /**
+ * @return the node ID with the namespace index of the server, or null
if its namespace URI is not in the
+ * namespace table of the server
+ */
+ private NodeId lookupMonitoredNode(final ExpandedNodeId nodeId) throws
Exception {
Review Comment:
Not blocking: this repeats the URI-or-index logic of `lookupNamespace`
below. Keeping it separate makes sense while the producers are out of scope.
When the producer follow-up is done, both could share one helper that handles
an unknown namespace URI.
##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/internal/SubscriptionManager.java:
##########
@@ -212,10 +210,14 @@ public List<PendingValue> putSubscriptions(final
Map<UInteger, Subscription> sub
for (final Map.Entry<UInteger, Subscription> entry :
subscriptions.entrySet()) {
final Subscription s = entry.getValue();
- final NodeId node = lookupNamespace(s.getNodeId()).get();
+ final NodeId node = lookupMonitoredNode(s.getNodeId());
if (node == null) {
- handleSubscriptionError(new
StatusCode(StatusCodes.Bad_InvalidArgument), entry.getKey(), s, pendingValues);
+ // reported once like a node the server rejects, the other
nodes and the connection are not affected
+ final StatusCode status = new
StatusCode(StatusCodes.Bad_NodeIdUnknown);
Review Comment:
Just noting a behaviour change, no action needed: before, the NPE rebuilt
the connection every `reconnectTimeout`, so a namespace the server registers
later was eventually picked up, though only by accident. Now the node is
reported once and is created again only when the route restarts or the
connection is rebuilt. That matches the #27780 design and the docs describe it.
--
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]
