davsclaus commented on code in PR #27279:
URL: https://github.com/apache/camel/pull/27279#discussion_r4165608802
##########
dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/WebSocketCliConnectorTransport.java:
##########
@@ -94,12 +94,15 @@ public class WebSocketCliConnectorTransport extends
ServiceSupport implements Cl
private CliWebSocketClient client;
private ThreadPoolExecutor actions;
private ScheduledExecutorService scheduler;
+ // collects snapshots: the status of a large integration can take seconds,
which must not hold up the scheduler
+ // (heartbeats, sends); snapshots are handed to the scheduler to be sent
+ private ScheduledExecutorService collector;
// fields below are only used from the scheduler thread, except the
volatile ones
Review Comment:
With this PR, `ticks`, `snapshotFuture` and `debugFuture` (below), and
`lastTraceUid` / `lastReceiveUid` / `lastSent` on `Connection`, are only used
on the collector thread, not the scheduler. Could you update this comment, the
one on the `Connection` fields, and the class javadoc ("connecting, parsing the
incoming frames, snapshots, heartbeats ... run on a second thread")? The class
relies on these comments to explain who owns what.
##########
dsl/camel-cli-connector/src/main/java/org/apache/camel/cli/connector/WebSocketCliConnectorTransport.java:
##########
@@ -491,7 +521,8 @@ private void sendSnapshot(Connection c, String kind,
JsonObject data) {
JsonObject frame = envelope("snapshot");
frame.put("kind", kind);
frame.put("data", data);
- send(c, frame);
+ // only the scheduler sends, one frame at a time
+ execute(() -> send(c, frame));
Review Comment:
Before this change, collecting and sending on one thread gave natural
backpressure. Now the next round waits for the collection time but not the send
time, and the collector keeps adding frames to the scheduler's unbounded queue,
including trace/receive batches of up to 128 KB each. On a slow link where
sends succeed but each takes seconds, the queue can keep growing, and pings
wait behind those sends. A failed send closes the connection and the rest of
the queue is then dropped, so it is self-limiting in the worst case.
Could the next round wait for the frames of this round to be sent? For
example, `snapshotRound` could queue a marker task on the scheduler after its
frames and schedule the next round from there, or it could skip a round while
frames are still pending (an `AtomicInteger` counted down in `send`).
--
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]
