Jetty 12 - Review Jetty WebSocket API batching

[sbordet]

Jetty version(s)
12+

Enhancement Description
After #9552, the batching in sends is hardcoded to disabled.

Is it worth to reintroduce it in the API?

There can be 3 proposals:

// Proposal 1 - Similar to WebSocket Core: boolean parameter.
session.send[Partial]Binary(ByteBuffer payload, boolean last, Callback callback, boolean batch); 

// Proposal 2 - boolean field on stateful Session similar to Jetty 11's Session.RemoteEndpoint.
session.setBatch(true);
session.send...(payload, last, callback);

// Proposal 3 - scoped batching
session.batch(() -> {
  session.send...(payload, last, callback);
  session.send...(payload, last, callback);
});

Proposal 2 & 3 are easier to deprecate if turns out to be a not so good idea, because we only deprecate 1 method rather than 4.

[sbordet]

@gregw @joakime @lachlan-roberts thoughts?

[gregw]

I'm not a huge fan of features that don't have known use-case, as it is hard to get it right unless there is something using it.

I think that if we do add a mechanism, not calling it batching may help describe it. Perhaps call it autoFlush, or lazyFlush?

Or maybe we just provide a gather send?

but my preference is to wait until somebody needs it.

[sbordet]

Agreed, closing for now.

[brettwooldridge]

In Jetty 9 I used the following annotation:

@WebSocket(batchMode = BatchMode.OFF, maxBinaryMessageSize = 1024 * 1024 * 4, maxIdleTime = Integer.MAX_VALUE)

With respect to batching, does this concept still exist? Or is it not something I need to be concerned with?

[joakime]

@brettwooldridge can you explain why do you use that feature? What you think it does?

[brettwooldridge]

My understanding, which could be mistaken, is that if batch mode was enabled in Jetty 9, that write operations performed on the websocket could result in bytes not being flushed until a threshold number of bytes has been reached, rather than immediately. This appears to be embodied in this code:

jetty.project/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/FrameFlusher.java

Line 193 in 86586df

return currentBatchMode == BatchMode.OFF ? flush() : batch();

Because the code in question was implemented some 7 years ago, I cannot recall whether we encountered a demonstrable issue or whether it was merely a concern of a developer. Because the websocket is used for realtime event notification, the issue or concern is that event delivery might be delayed by batching behavior.

[lachlan-roberts]

@brettwooldridge the batching of frames is still a concept in Jettys lower level websocket-core API. But there is just no configuration for it in the new jetty-websocket-api, it is always off and there is no option to enable it.

So you will not need this batchMode configuration anymore as it is always off.
@WebSocket(batchMode = BatchMode.OFF