Projects tigase _server server-core Issues #1324
StreamManagement sends request for ack for each packet (#1324)
Closed
Andrzej Wójcik (Tigase) opened 2 years ago

When no. of unacked stanzas in the Stream Management queue is bigger than configured ack_request_count then the server sends <r/> packet for each new packet sent to the client. This is most likely overkill, increases data traffic sent to the client, and duplicates no. of stanza sent to the client (ie. mobile device with poor connectivity).

Moreover, this forces client to send <a/> response stanza for each received <r/> stanza, causing increased traffic sent to the XMPP server.

All of that with long round trip time between client and the server, causes saturation of small network buffer used by the server for clients connections and increases delay, ie. during MAM sync.

Andrzej Wójcik (Tigase) commented 2 years ago

The logic of StreamManagementIOProcessor::shouldRequestAck() was modified to follow this rules:

  • sending <r/> only for X unacked stanzas after the previous <r/> request sent to the client If ack_request_count is set to 10, then for 100 messages sent in a batch, it would result in 10 <r/> sent (assuming no. <a/> was received during this time).
  • sending <r/> only if it was not sent in the last X milliseconds and if during this time no <a/> was received This would even further reduce no. of requests sent for stanzas sent in a batch as by default it would send only a single <r/> for all stanzas in a batch which were sent in 200ms (ack_request_min_delay is configurable property).

By setting ack_request_min_delay to 0, the server would use only the stanza counter for estimating when it should send requests. By setting also ack_request_count to 1, the server would be again sending <r/> for each sent stanza.

This new code is available in a feature branch and PR was created at https://github.com/tigase/tigase-server/pull/150

@wojtek Please review this logic and code before we merge this to master.

wojciech.kapcia@tigase.net commented 2 years ago

I reviewed the code and it looks ok. Added a couple minute comments. Please also include this in the documentation (could be based on your comment above)

issue 1 of 1
Type
Bug
Priority
Normal
Assignee
Version
tigase-server-8.3.0
Spent time
1h 45m
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#1324
Please wait...
Page is in error, reload to recover