fix and optimize RTO computation

fixes #144

ackhandler/packet.go

@@ -19,7 +19,6 @@ type Packet struct {
 	Retransmitted  bool // has this Packet ever been retransmitted
 
 	sendTime time.Time
-	rtoTime  time.Time
 }
 
 // GetStreamFramesForRetransmission gets all the streamframes for retransmission

ackhandler/sent_packet_handler.go

@@ -19,7 +19,7 @@ var (
 	// ErrMapAccess occurs when a NACK contains invalid NACK ranges
 	ErrMapAccess = qerr.Error(qerr.InvalidAckData, "Packet does not exist in PacketHistory")
 	// ErrTooManyTrackedSentPackets occurs when the sentPacketHandler has to keep track of too many packets
-	ErrTooManyTrackedSentPackets = errors.New("To many outstanding not-acked and not retransmitted packets.")
+	ErrTooManyTrackedSentPackets = errors.New("Too many outstanding non-acked and non-retransmitted packets")
 	errAckForUnsentPacket        = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package")
 )
 
@@ -31,6 +31,7 @@ var (
 type sentPacketHandler struct {
 	lastSentPacketNumber            protocol.PacketNumber
 	lastSentPacketEntropy           EntropyAccumulator
+	lastSentPacketTime              time.Time
 	highestInOrderAckedPacketNumber protocol.PacketNumber
 	LargestObserved                 protocol.PacketNumber
 	LargestObservedEntropy          EntropyAccumulator
@@ -118,8 +119,8 @@ func (h *sentPacketHandler) SentPacket(packet *Packet) error {
 		return errWrongPacketNumberIncrement
 	}
 	now := time.Now()
+	h.lastSentPacketTime = now
 	packet.sendTime = now
-	packet.rtoTime = now.Add(h.getRTO())
 	if packet.Length == 0 {
 		return errors.New("SentPacketHandler: packet cannot be empty")
 	}
@@ -250,7 +251,7 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *frames.AckFrame) error {
 }
 
 func (h *sentPacketHandler) HasPacketForRetransmission() bool {
-	h.queuePacketsRTO()
+	h.maybeQueuePacketsRTO()
 
 	if len(h.retransmissionQueue) > 0 {
 		return true
@@ -290,6 +291,20 @@ func (h *sentPacketHandler) CheckForError() error {
 	return nil
 }
 
+func (h *sentPacketHandler) maybeQueuePacketsRTO() {
+	if time.Now().Before(h.TimeOfFirstRTO()) {
+		return
+	}
+	for p := h.highestInOrderAckedPacketNumber + 1; p <= h.lastSentPacketNumber; p++ {
+		packet := h.packetHistory[p]
+		if packet != nil && !packet.Retransmitted {
+			h.queuePacketForRetransmission(packet)
+			h.congestion.OnRetransmissionTimeout(true)
+			return
+		}
+	}
+}
+
 func (h *sentPacketHandler) getRTO() time.Duration {
 	rto := h.congestion.RetransmissionDelay()
 	if rto == 0 {
@@ -298,30 +313,9 @@ func (h *sentPacketHandler) getRTO() time.Duration {
 	return utils.MaxDuration(rto, protocol.MinRetransmissionTime)
 }
 
-func (h *sentPacketHandler) queuePacketsRTO() {
-	queued := false
-	now := time.Now()
-	for _, p := range h.packetHistory {
-		if p == nil || p.Retransmitted || p.rtoTime.After(now) {
-			continue
-		}
-		h.queuePacketForRetransmission(p)
-		queued = true
-	}
-	if queued {
-		h.congestion.OnRetransmissionTimeout(true)
-	}
-}
-
 func (h *sentPacketHandler) TimeOfFirstRTO() time.Time {
-	var min time.Time
+	if h.lastSentPacketTime.IsZero() {
-	for _, p := range h.packetHistory {
+		return time.Time{}
-		if p == nil || p.Retransmitted {
-			continue
-		}
-		if min.IsZero() || min.After(p.rtoTime) {
-			min = p.rtoTime
-		}
 	}
-	return min
+	return h.lastSentPacketTime.Add(h.getRTO())
 }

ackhandler/sent_packet_handler_test.go

@@ -6,7 +6,6 @@ import (
 	"github.com/lucas-clemente/quic-go/congestion"
 	"github.com/lucas-clemente/quic-go/frames"
 	"github.com/lucas-clemente/quic-go/protocol"
-	"github.com/lucas-clemente/quic-go/utils"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -160,6 +159,13 @@ var _ = Describe("SentPacketHandler", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(handler.packetHistory[1].sendTime.Unix()).To(BeNumerically("~", time.Now().Unix(), 1))
 		})
+
+		It("updates the last sent time", func() {
+			packet := Packet{PacketNumber: 1, Frames: []frames.Frame{&streamFrame}, EntropyBit: true, Length: 1}
+			err := handler.SentPacket(&packet)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.lastSentPacketTime.Unix()).To(BeNumerically("~", time.Now().Unix(), 1))
+		})
 	})
 
 	Context("DOS mitigation", func() {
@@ -524,7 +530,7 @@ var _ = Describe("SentPacketHandler", func() {
 
 			// Simulate protocol.RetransmissionThreshold more NACKs
 			for i := uint8(0); i < protocol.RetransmissionThreshold; i++ {
-				_, err := handler.nackPacket(1)
+				_, err = handler.nackPacket(1)
 				Expect(err).ToNot(HaveOccurred())
 			}
 			Expect(handler.BytesInFlight()).To(Equal(protocol.ByteCount(0)))
@@ -612,8 +618,8 @@ var _ = Describe("SentPacketHandler", func() {
 		It("should call OnRetransmissionTimeout", func() {
 			err := handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1})
 			Expect(err).NotTo(HaveOccurred())
-			handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second)
+			handler.lastSentPacketTime = time.Now().Add(-time.Second)
-			handler.queuePacketsRTO()
+			handler.maybeQueuePacketsRTO()
 			Expect(cong.onRetransmissionTimeout).To(BeTrue())
 		})
 	})
@@ -635,13 +641,6 @@ var _ = Describe("SentPacketHandler", func() {
 			handler.rttStats.UpdateRTT(rtt, 0, time.Now())
 			Expect(handler.getRTO()).To(Equal(protocol.MinRetransmissionTime))
 		})
-
-		It("stores RTO in sent packets", func() {
-			handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1})
-			val := handler.packetHistory[1].rtoTime
-			expected := time.Now().Add(protocol.DefaultRetransmissionTime)
-			Expect(utils.AbsDuration(expected.Sub(val))).To(BeNumerically("<", time.Millisecond))
-		})
 	})
 
 	Context("RTO retransmission", func() {
@@ -658,7 +657,7 @@ var _ = Describe("SentPacketHandler", func() {
 
 			It("ignores nil packets", func() {
 				handler.packetHistory[1] = nil
-				handler.queuePacketsRTO()
+				handler.maybeQueuePacketsRTO()
 				Expect(handler.TimeOfFirstRTO().IsZero()).To(BeTrue())
 			})
 		})
@@ -667,7 +666,7 @@ var _ = Describe("SentPacketHandler", func() {
 			It("does nothing if not required", func() {
 				err := handler.SentPacket(&Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1})
 				Expect(err).NotTo(HaveOccurred())
-				handler.queuePacketsRTO()
+				handler.maybeQueuePacketsRTO()
 				Expect(handler.retransmissionQueue).To(BeEmpty())
 			})
 
@@ -675,8 +674,8 @@ var _ = Describe("SentPacketHandler", func() {
 				p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1}
 				err := handler.SentPacket(p)
 				Expect(err).NotTo(HaveOccurred())
-				handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second)
+				handler.lastSentPacketTime = time.Now().Add(-time.Second)
-				handler.queuePacketsRTO()
+				handler.maybeQueuePacketsRTO()
 				Expect(handler.retransmissionQueue).To(HaveLen(1))
 				Expect(handler.retransmissionQueue[0]).To(Equal(p))
 			})
@@ -685,14 +684,14 @@ var _ = Describe("SentPacketHandler", func() {
 				p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1, Retransmitted: true}
 				err := handler.SentPacket(p)
 				Expect(err).NotTo(HaveOccurred())
-				handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second)
+				handler.lastSentPacketTime = time.Now().Add(-time.Second)
-				handler.queuePacketsRTO()
+				handler.maybeQueuePacketsRTO()
 				Expect(handler.retransmissionQueue).To(BeEmpty())
 			})
 
 			It("ignores nil packets", func() {
 				handler.packetHistory[1] = nil
-				handler.queuePacketsRTO()
+				handler.maybeQueuePacketsRTO()
 				Expect(handler.retransmissionQueue).To(BeEmpty())
 			})
 		})
@@ -701,7 +700,7 @@ var _ = Describe("SentPacketHandler", func() {
 			p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1}
 			err := handler.SentPacket(p)
 			Expect(err).NotTo(HaveOccurred())
-			handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second)
+			handler.lastSentPacketTime = time.Now().Add(-time.Second)
 			Expect(handler.HasPacketForRetransmission()).To(BeTrue())
 		})
 
@@ -709,7 +708,7 @@ var _ = Describe("SentPacketHandler", func() {
 			p := &Packet{PacketNumber: 1, Frames: []frames.Frame{}, Length: 1}
 			err := handler.SentPacket(p)
 			Expect(err).NotTo(HaveOccurred())
-			handler.packetHistory[1].rtoTime = time.Now().Add(-time.Second)
+			handler.lastSentPacketTime = time.Now().Add(-time.Second)
 			Expect(handler.DequeuePacketForRetransmission()).To(Equal(p))
 		})
 	})