From 2056960e07dc68640a4aaab2d8f02dfe6e00a49a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 27 Apr 2016 17:19:54 +0700 Subject: [PATCH] only delete a Packet from history in SentPacketHandler once it has been ACKed --- ackhandler/packet.go | 1 + ackhandler/sent_packet_handler.go | 19 +++++++------------ ackhandler/sent_packet_handler_test.go | 5 +++-- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/ackhandler/packet.go b/ackhandler/packet.go index 47252f7ec..008e6c2d5 100644 --- a/ackhandler/packet.go +++ b/ackhandler/packet.go @@ -13,4 +13,5 @@ type Packet struct { Entropy EntropyAccumulator MissingReports uint8 + Retransmitted bool // has this Packet ever been retransmitted } diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index 3e1400fb7..417077def 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -50,6 +50,12 @@ func (h *sentPacketHandler) nackPacket(packetNumber protocol.PacketNumber) error return ErrMapAccess } + // if the packet has already been retransmit, do nothing + // we're probably only receiving another NACK for this packet because the retransmission has not yet arrived at the client + if packet.Retransmitted { + return nil + } + packet.MissingReports++ if packet.MissingReports > retransmissionThreshold { @@ -58,20 +64,9 @@ func (h *sentPacketHandler) nackPacket(packetNumber protocol.PacketNumber) error return nil } -func (h *sentPacketHandler) recalculateHighestInOrderAckedPacketNumberFromPacketHistory() { - for i := h.highestInOrderAckedPacketNumber; i <= h.lastSentPacketNumber; i++ { - _, ok := h.packetHistory[i] - if ok { - break - } - h.highestInOrderAckedPacketNumber = i - } -} - func (h *sentPacketHandler) queuePacketForRetransmission(packet *Packet) { h.retransmissionQueue = append(h.retransmissionQueue, packet) - h.ackPacket(packet.PacketNumber) - h.recalculateHighestInOrderAckedPacketNumberFromPacketHistory() + packet.Retransmitted = true } func (h *sentPacketHandler) SentPacket(packet *Packet) error { diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index 8d1e8ea3a..a4c489b8d 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -340,15 +340,16 @@ var _ = Describe("SentPacketHandler", func() { Expect(handler.DequeuePacketForRetransmission()).To(BeNil()) }) - It("recalculates the highestInOrderAckedPacketNumber after queueing a retransmission", func() { + It("does not change the highestInOrderAckedPacketNumber after queueing a retransmission", func() { ack := frames.AckFrame{ LargestObserved: 4, NackRanges: []frames.NackRange{frames.NackRange{FirstPacketNumber: 3, LastPacketNumber: 3}}, } err := handler.ReceivedAck(&ack) Expect(err).ToNot(HaveOccurred()) + Expect(handler.highestInOrderAckedPacketNumber).To(Equal(protocol.PacketNumber(2))) handler.nackPacket(3) // this is the second NACK for this packet - Expect(handler.highestInOrderAckedPacketNumber).To(Equal(protocol.PacketNumber(4))) + Expect(handler.highestInOrderAckedPacketNumber).To(Equal(protocol.PacketNumber(2))) }) }) })