Skip to content

Commit

Permalink
rename retransmittable to ack-eliciting
Browse files Browse the repository at this point in the history
  • Loading branch information
marten-seemann committed Apr 12, 2019
1 parent 9ad6a7b commit b5336be
Show file tree
Hide file tree
Showing 14 changed files with 178 additions and 177 deletions.
34 changes: 34 additions & 0 deletions internal/ackhandler/ack_eliciting.go
@@ -0,0 +1,34 @@
package ackhandler

import "github.com/lucas-clemente/quic-go/internal/wire"

// Returns a new slice with all non-ack-eliciting frames deleted.
func stripNonAckElicitingFrames(fs []wire.Frame) []wire.Frame {
res := make([]wire.Frame, 0, len(fs))
for _, f := range fs {
if IsFrameAckEliciting(f) {
res = append(res, f)
}
}
return res
}

// IsFrameAckEliciting returns true if the frame is ack-eliciting.
func IsFrameAckEliciting(f wire.Frame) bool {
switch f.(type) {
case *wire.AckFrame:
return false
default:
return true
}
}

// HasAckElicitingFrames returns true if at least one frame is ack-eliciting.
func HasAckElicitingFrames(fs []wire.Frame) bool {
for _, f := range fs {
if IsFrameAckEliciting(f) {
return true
}
}
return false
}
Expand Up @@ -8,7 +8,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = Describe("retransmittable frames", func() {
var _ = Describe("ack-eliciting frames", func() {
for fl, el := range map[wire.Frame]bool{
&wire.AckFrame{}: false,
&wire.DataBlockedFrame{}: true,
Expand All @@ -24,20 +24,20 @@ var _ = Describe("retransmittable frames", func() {
fName := reflect.ValueOf(f).Elem().Type().Name()

It("works for "+fName, func() {
Expect(IsFrameRetransmittable(f)).To(Equal(e))
Expect(IsFrameAckEliciting(f)).To(Equal(e))
})

It("stripping non-retransmittable frames works for "+fName, func() {
It("stripping non-ack-elicinting frames works for "+fName, func() {
s := []wire.Frame{f}
if e {
Expect(stripNonRetransmittableFrames(s)).To(Equal([]wire.Frame{f}))
Expect(stripNonAckElicitingFrames(s)).To(Equal([]wire.Frame{f}))
} else {
Expect(stripNonRetransmittableFrames(s)).To(BeEmpty())
Expect(stripNonAckElicitingFrames(s)).To(BeEmpty())
}
})

It("HasRetransmittableFrames works for "+fName, func() {
Expect(HasRetransmittableFrames([]wire.Frame{f})).To(Equal(e))
It("HasAckElicitingFrames works for "+fName, func() {
Expect(HasAckElicitingFrames([]wire.Frame{f})).To(Equal(e))
})
}
})
10 changes: 5 additions & 5 deletions internal/ackhandler/received_packet_handler.go
Expand Up @@ -11,12 +11,12 @@ import (
)

const (
// maximum delay that can be applied to an ACK for a retransmittable packet
// maximum delay that can be applied to an ACK for an ack-eliciting packet
ackSendDelay = 25 * time.Millisecond
// initial maximum number of retransmittable packets received before sending an ack.
initialRetransmittablePacketsBeforeAck = 2
// number of retransmittable that an ACK is sent for
retransmittablePacketsBeforeAck = 10
// initial maximum number of ack-eliciting packets received before sending an ack.
initialAckElicitingPacketsBeforeAck = 2
// number of ack-eliciting that an ACK is sent for
ackElicitingPacketsBeforeAck = 10
// 1/5 RTT delay when doing ack decimation
ackDecimationDelay = 1.0 / 4
// 1/8 RTT delay when doing ack decimation
Expand Down
24 changes: 12 additions & 12 deletions internal/ackhandler/received_packet_tracker.go
Expand Up @@ -19,11 +19,11 @@ type receivedPacketTracker struct {
ackSendDelay time.Duration
rttStats *congestion.RTTStats

packetsReceivedSinceLastAck int
retransmittablePacketsReceivedSinceLastAck int
ackQueued bool
ackAlarm time.Time
lastAck *wire.AckFrame
packetsReceivedSinceLastAck int
ackElicitingPacketsReceivedSinceLastAck int
ackQueued bool
ackAlarm time.Time
lastAck *wire.AckFrame

logger utils.Logger

Expand Down Expand Up @@ -115,14 +115,14 @@ func (h *receivedPacketTracker) maybeQueueAck(packetNumber protocol.PacketNumber
}

if !h.ackQueued && shouldInstigateAck {
h.retransmittablePacketsReceivedSinceLastAck++
h.ackElicitingPacketsReceivedSinceLastAck++

if packetNumber > minReceivedBeforeAckDecimation {
// ack up to 10 packets at once
if h.retransmittablePacketsReceivedSinceLastAck >= retransmittablePacketsBeforeAck {
if h.ackElicitingPacketsReceivedSinceLastAck >= ackElicitingPacketsBeforeAck {
h.ackQueued = true
if h.logger.Debug() {
h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using threshold: %d).", h.retransmittablePacketsReceivedSinceLastAck, retransmittablePacketsBeforeAck)
h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using threshold: %d).", h.ackElicitingPacketsReceivedSinceLastAck, ackElicitingPacketsBeforeAck)
}
} else if h.ackAlarm.IsZero() {
// wait for the minimum of the ack decimation delay or the delayed ack time before sending an ack
Expand All @@ -133,10 +133,10 @@ func (h *receivedPacketTracker) maybeQueueAck(packetNumber protocol.PacketNumber
}
}
} else {
// send an ACK every 2 retransmittable packets
if h.retransmittablePacketsReceivedSinceLastAck >= initialRetransmittablePacketsBeforeAck {
// send an ACK every 2 ack-eliciting packets
if h.ackElicitingPacketsReceivedSinceLastAck >= initialAckElicitingPacketsBeforeAck {
if h.logger.Debug() {
h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using initial threshold: %d).", h.retransmittablePacketsReceivedSinceLastAck, initialRetransmittablePacketsBeforeAck)
h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using initial threshold: %d).", h.ackElicitingPacketsReceivedSinceLastAck, initialAckElicitingPacketsBeforeAck)
}
h.ackQueued = true
} else if h.ackAlarm.IsZero() {
Expand Down Expand Up @@ -184,7 +184,7 @@ func (h *receivedPacketTracker) GetAckFrame() *wire.AckFrame {
h.ackAlarm = time.Time{}
h.ackQueued = false
h.packetsReceivedSinceLastAck = 0
h.retransmittablePacketsReceivedSinceLastAck = 0
h.ackElicitingPacketsReceivedSinceLastAck = 0
return ack
}

Expand Down
8 changes: 4 additions & 4 deletions internal/ackhandler/received_packet_tracker_test.go
Expand Up @@ -108,7 +108,7 @@ var _ = Describe("Received Packet Tracker", func() {
Expect(tracker.GetAckFrame().DelayTime).To(BeNumerically("~", 0, time.Second))
})

It("queues an ACK for every second retransmittable packet at the beginning", func() {
It("queues an ACK for every second ack-eliciting packet at the beginning", func() {
receiveAndAck10Packets()
p := protocol.PacketNumber(11)
for i := 0; i <= 20; i++ {
Expand All @@ -125,7 +125,7 @@ var _ = Describe("Received Packet Tracker", func() {
}
})

It("queues an ACK for every 10 retransmittable packet, if they are arriving fast", func() {
It("queues an ACK for every 10 ack-eliciting packet, if they are arriving fast", func() {
receiveAndAck10Packets()
p := protocol.PacketNumber(10000)
for i := 0; i < 9; i++ {
Expand All @@ -141,7 +141,7 @@ var _ = Describe("Received Packet Tracker", func() {
Expect(tracker.GetAlarmTimeout()).To(BeZero())
})

It("only sets the timer when receiving a retransmittable packets", func() {
It("only sets the timer when receiving a ack-eliciting packets", func() {
receiveAndAck10Packets()
err := tracker.ReceivedPacket(11, time.Now(), false)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -350,7 +350,7 @@ var _ = Describe("Received Packet Tracker", func() {
Expect(tracker.GetAckFrame()).ToNot(BeNil())
Expect(tracker.packetsReceivedSinceLastAck).To(BeZero())
Expect(tracker.GetAlarmTimeout()).To(BeZero())
Expect(tracker.retransmittablePacketsReceivedSinceLastAck).To(BeZero())
Expect(tracker.ackElicitingPacketsReceivedSinceLastAck).To(BeZero())
Expect(tracker.ackQueued).To(BeFalse())
})

Expand Down
34 changes: 0 additions & 34 deletions internal/ackhandler/retransmittable.go

This file was deleted.

26 changes: 13 additions & 13 deletions internal/ackhandler/sent_packet_handler.go
Expand Up @@ -37,8 +37,8 @@ func newPacketNumberSpace(initialPN protocol.PacketNumber) *packetNumberSpace {
}

type sentPacketHandler struct {
lastSentRetransmittablePacketTime time.Time // only applies to the application-data packet number space
lastSentCryptoPacketTime time.Time
lastSentAckElicitingPacketTime time.Time // only applies to the application-data packet number space
lastSentCryptoPacketTime time.Time

nextSendTime time.Time

Expand Down Expand Up @@ -125,7 +125,7 @@ func (h *sentPacketHandler) SetHandshakeComplete() {
}

func (h *sentPacketHandler) SentPacket(packet *Packet) {
if isRetransmittable := h.sentPacketImpl(packet); isRetransmittable {
if isAckEliciting := h.sentPacketImpl(packet); isAckEliciting {
h.getPacketNumberSpace(packet.EncryptionLevel).history.SentPacket(packet)
h.updateLossDetectionAlarm()
}
Expand All @@ -134,7 +134,7 @@ func (h *sentPacketHandler) SentPacket(packet *Packet) {
func (h *sentPacketHandler) SentPacketsAsRetransmission(packets []*Packet, retransmissionOf protocol.PacketNumber) {
var p []*Packet
for _, packet := range packets {
if isRetransmittable := h.sentPacketImpl(packet); isRetransmittable {
if isAckEliciting := h.sentPacketImpl(packet); isAckEliciting {
p = append(p, packet)
}
}
Expand All @@ -155,7 +155,7 @@ func (h *sentPacketHandler) getPacketNumberSpace(encLevel protocol.EncryptionLev
}
}

func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* isRetransmittable */ {
func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* is ack-eliciting */ {
pnSpace := h.getPacketNumberSpace(packet.EncryptionLevel)

if h.logger.Debug() && pnSpace.largestSent != 0 {
Expand All @@ -172,25 +172,25 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* isRetransmitt
}
}

packet.Frames = stripNonRetransmittableFrames(packet.Frames)
isRetransmittable := len(packet.Frames) != 0
packet.Frames = stripNonAckElicitingFrames(packet.Frames)
isAckEliciting := len(packet.Frames) != 0

if isRetransmittable {
if isAckEliciting {
if packet.EncryptionLevel != protocol.Encryption1RTT {
h.lastSentCryptoPacketTime = packet.SendTime
}
h.lastSentRetransmittablePacketTime = packet.SendTime
h.lastSentAckElicitingPacketTime = packet.SendTime
packet.includedInBytesInFlight = true
h.bytesInFlight += packet.Length
packet.canBeRetransmitted = true
if h.numProbesToSend > 0 {
h.numProbesToSend--
}
}
h.congestion.OnPacketSent(packet.SendTime, h.bytesInFlight, packet.PacketNumber, packet.Length, isRetransmittable)
h.congestion.OnPacketSent(packet.SendTime, h.bytesInFlight, packet.PacketNumber, packet.Length, isAckEliciting)

h.nextSendTime = utils.MaxTime(h.nextSendTime, packet.SendTime).Add(h.congestion.TimeUntilSend(h.bytesInFlight))
return isRetransmittable
return isAckEliciting
}

func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumber protocol.PacketNumber, encLevel protocol.EncryptionLevel, rcvTime time.Time) error {
Expand Down Expand Up @@ -325,7 +325,7 @@ func (h *sentPacketHandler) updateLossDetectionAlarm() {
// Early retransmit timer or time loss detection.
h.alarm = h.lossTime
} else { // PTO alarm
h.alarm = h.lastSentRetransmittablePacketTime.Add(h.computePTOTimeout())
h.alarm = h.lastSentAckElicitingPacketTime.Add(h.computePTOTimeout())
}
}

Expand Down Expand Up @@ -438,7 +438,7 @@ func (h *sentPacketHandler) onPacketAcked(p *Packet, rcvTime time.Time) error {
}

// only report the acking of this packet to the congestion controller if:
// * it is a retransmittable packet
// * it is an ack-eliciting packet
// * this packet wasn't retransmitted yet
if p.isRetransmission {
// that the parent doesn't exist is expected to happen every time the original packet was already acked
Expand Down

0 comments on commit b5336be

Please sign in to comment.