Opened 8 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#2706 closed enhancement (fixed)

Reduce DEFAULT_INITIAL_ACK_DELAY

Reported by: Zlatin Balevsky Owned by:
Priority: minor Milestone: 0.9.46
Component: streaming Version: 0.9.45
Keywords: testnet Cc:
Parent Tickets: Sensitive: no

Description

I did an experiment of reducing this value drastically. To my surprise, it did not result in reduction in the efficiency of acks as measured by average messages per ACK.

In the attached spreadsheet there are three pages:
"Throughput" measures final throughput as reported by eepget
"Ack efficiency 50" lists the number of messages per ACK as extracted from logs of DEFAULT_INITIAL_ACK_DELAY=50
"Ack efficiency 750" - same as above but with default value of 750

Subtickets

Attachments (6)

Default_ack_delay.ods (12.9 KB) - added by Zlatin Balevsky 8 weeks ago.
ack-metrics-vanilla.csv (1.4 KB) - added by Zlatin Balevsky 8 weeks ago.
CSV file with numberAcked,highestRTT,RTT,RTO for vanilla
ack-metrics-50x50.csv (1.6 KB) - added by Zlatin Balevsky 8 weeks ago.
CSV file with numberAcked,highestRTT,RTT,RTO for 50x50
window-vanilla.csv (1.5 KB) - added by Zlatin Balevsky 8 weeks ago.
CSV file with wsize,cwin,rtt,rto,unackedOut for vanilla
window-50x50.csv (1.7 KB) - added by Zlatin Balevsky 8 weeks ago.
CSV file with wsize,cwin,rtt,rto,unackedOut for 50x50
window-charts.ods (50.7 KB) - added by Zlatin Balevsky 8 weeks ago.
Charts of the window metrics, data taken from CSV files attached to this ticket

Download all attachments as: .zip

Change History (16)

Changed 8 weeks ago by Zlatin Balevsky

Attachment: Default_ack_delay.ods added

Changed 8 weeks ago by Zlatin Balevsky

Attachment: ack-metrics-vanilla.csv added

CSV file with numberAcked,highestRTT,RTT,RTO for vanilla

Changed 8 weeks ago by Zlatin Balevsky

Attachment: ack-metrics-50x50.csv added

CSV file with numberAcked,highestRTT,RTT,RTO for 50x50

Changed 8 weeks ago by Zlatin Balevsky

Attachment: window-vanilla.csv added

CSV file with wsize,cwin,rtt,rto,unackedOut for vanilla

Changed 8 weeks ago by Zlatin Balevsky

Attachment: window-50x50.csv added

CSV file with wsize,cwin,rtt,rto,unackedOut for 50x50

Changed 8 weeks ago by Zlatin Balevsky

Attachment: window-charts.ods added

Charts of the window metrics, data taken from CSV files attached to this ticket

comment:1 Changed 8 weeks ago by zzz

ConnectionOptions?.java
history: reduced from 2000 to 1000 by you in 2013;
reduced from 1000 to 750 by me in 2015
One reason it's high is that standalone acks are so bandwidth-expensive compared to TCP, because of the garlic wrapping.

Reducing it may improve latency (reduce measured RTT/RTO) on one-way type connections (where all the data are flowing one way, like HTTP GET).

In TCP, we know what the other end's send window is and can ack immediately if it's full. In streaming we don't know. I thought we had some logic to ack immediately if there were too many unacked inbound packets but I can't find it.

I think we could set FLAG_OPTIONAL_DELAY with a delay of 0 to request an immediate ack when we're at the end of our send window, that might help. Look for ackImmediately() in ConnectionPacketHandler?.

One other issue is it's called "initial" ack delay but it's never changed. setSendAckDelay() is unused. Seems like it should be a function of RTT or RTO or something after we get going.

Haven't looked at attachments yet.

comment:2 in reply to:  1 Changed 8 weeks ago by zzz

Replying to zzz:

I think we could set FLAG_OPTIONAL_DELAY with a delay of 0 to request an immediate ack when we're at the end of our send window, that might help. Look for ackImmediately() in ConnectionPacketHandler?.

Found code to do this in Connection.java line 396, so I think we're good there. Since we do that, the effects of the OP change may be minimal. Perhaps lowering RTT if we're well below the window? Not sure…

comment:3 Changed 8 weeks ago by zzz

IMMEDIATE_ACK_DELAY in CPH is 150, references tickets #1939 and #2584, reduced from 250 to 150 last year, may also be due for another reduction

comment:4 Changed 7 weeks ago by zzz

Version: 0.9.460.9.45

comment:5 Changed 6 weeks ago by zzz

Won't matter if we reduce DEFAULT_INITIAL_ACK_DELAY to less than MIN_RESEND_DELAY, but we shouldn't have the value be higher than RTO:

--- apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionOptions.java	3e7f000737c3fa4d2e283f1c8648cd022932aff7
+++ apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionOptions.java	e5459556ea701f04d3cb753c33d905ad1af82ae8
@@ -706,9 +711,9 @@ class ConnectionOptions extends I2PSocke
      *
      * @return ACK delay in ms
      */
-    public int getSendAckDelay() { return _sendAckDelay; }
+    public synchronized int getSendAckDelay() { return Math.min(_sendAckDelay, _rto); }
     /**
-     *  Unused except here, so expect the default initial delay of 2000 ms unless set by the user
+     *  Unused except here, so expect the default initial delay of DEFAULT_INITIAL_ACK_DELAY unless set by the user
      *  to remain constant.
      */
     public void setSendAckDelay(int delayMs) { _sendAckDelay = delayMs; }

comment:6 Changed 6 weeks ago by zzz

or maybe even _rto / 2 or _rtt / 2

comment:7 Changed 6 weeks ago by zzz

Also proposed by me in #1939 comment 13

comment:8 Changed 3 weeks ago by zzz

The point of a relatively high initial ack delay is to allow the client application (generally a web server) to respond, so we don't send an expensive (both in bandwidth at the streaming layer, and CPU at the SKM layer) empty SYN ACK with no payload. In theory, if the client application responds before this, then it doesn't matter.

750 does sound too high as an initial value.

However, as noted above, setSendAckDelay() is unused, so the initial delay is always the value.
We only set an OptionalDelay? value in a packet of 0 (for immediate ack) or SEND_DELAY_CHOKE (for choking), so there's never any intermediate value that represents anything.

The major place getSendAckDelay() is used is at CPH line 206.

RFC 5681 section 4.3 and RFC 1122 section 4.2.3.2 give the guidance about delayed acks. The delay MUST be less than 500 ms and SHOULD be no more than two window sizes.

As noted above in comments 2 and 3, unlike TCP, streaming receiver does not 'know' the size of sender's send window. We work around that by sending an immediate ack request (OptionalDelay? value of 0) when we're near the end of our send window (Connection.java line 418). That's why changing the setting had little effect in tests. We can investigate whether the heuristics in the code there are sufficient to prevent stalls or not and tune them if necessary.

tl;dr in favor of reducing 750 to 500 now, and in the future, investigating if we have stalls and can reduce them by changing when we request an immediate ack.

comment:9 Changed 3 weeks ago by zzz

Milestone: undecided0.9.46
Resolution: fixed
Status: newclosed

comment:10 Changed 3 weeks ago by zzz

Note: the investigation and adjustment of the heuristics is existing ticket #2705

Note: See TracTickets for help on using tickets.