[GitHub] [activemq-artemis] gtully commented on a change in pull request #3566: ARTEMIS-3282 Expose replication response batching tuning

[GitBox <gi...@apache.org>]

gtully commented on a change in pull request #3566:
URL: https://github.com/apache/activemq-artemis/pull/3566#discussion_r630953529



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationEndpoint.java
##########
@@ -221,6 +221,8 @@ public void handlePacket(final Packet packet) {
                handleCommitRollback((ReplicationCommitMessage) packet);
                break;
             case PacketImpl.REPLICATION_PAGE_WRITE:
+               // potential blocking I/O operation! flush existing packets to save long tail latency
+               endOfBatch();

Review comment:
       I think if blocking ops are removed, there is no need to eagerly flush the batch here. However I really don't see how a user can choose a value for maxReplicaResponseBatchBytes. I think it has to be automatic or automagical, based on the some limit on what can be read.
   5.x has optimiseAck on by default, at 70% of the prefetch for openwire consumers. That covers the auto ack case and only sends an actual ack every X non persistent messages. I wonder if something similar based on confirmation-window could work here. Tho the endpoint probably has no way of knowing what is set on the other end I guess. Is there a relationship between the confirmation window and responses already?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org