mirror of
https://github.com/facebookincubator/mvfst.git
synced 2025-11-25 15:43:13 +03:00
Adjusted delay computation should flag long delay error only for ACK delays
Summary: the helper function `Decode.cpp:computeAdjustedDelay()` function takes a delay value, applies an exponent adjustment, and performs checks for overflow and validity before returning the adjusted delay. This function is used to decode both the ack_delay, as well as the relative time-gap between RX time-stamps. Additional logging collected via D41988681 and multiple long-running canaries indicate the "long ack delay error" that is flagged when the decoded value > 1000secs, occurs only when decoding rx timestamps. The first rx timestamp in any ack_receive_timestamp range has a relative delay computed with the connection start time as a basis, hence on long lived connections this can exceed > 1000secs (the current threshold for the error message). Therefore, we have moved the error statement (and the error check) to be only at the call site processing ack delays (and not rx timestamp relative delays). Reviewed By: bschlinker, mjoras Differential Revision: D46059268 fbshipit-source-id: 2637986d5919207b6c7d858a69b33139dad785e7
This commit is contained in:
committed by
Facebook GitHub Bot
parent
54155e668d
commit
d42d303e9d
@@ -144,30 +144,27 @@ ImmediateAckFrame decodeImmediateAckFrame(folly::io::Cursor&) {
|
||||
return ImmediateAckFrame();
|
||||
}
|
||||
|
||||
uint64_t computeAdjustedDelay(
|
||||
uint64_t convertEncodedDurationToMicroseconds(
|
||||
FrameType frameType,
|
||||
uint8_t exponentToUse,
|
||||
folly::Optional<std::pair<uint64_t, size_t>> delay) {
|
||||
uint64_t delay) {
|
||||
// ackDelayExponentToUse is guaranteed to be less than the size of uint64_t
|
||||
uint64_t delayOverflowMask = 0xFFFFFFFFFFFFFFFF;
|
||||
uint8_t leftShift = (sizeof(delay->first) * 8 - exponentToUse);
|
||||
uint8_t leftShift = (sizeof(delay) * 8 - exponentToUse);
|
||||
DCHECK_LT(leftShift, sizeof(delayOverflowMask) * 8);
|
||||
delayOverflowMask = delayOverflowMask << leftShift;
|
||||
if ((delay->first & delayOverflowMask) != 0) {
|
||||
if ((delay & delayOverflowMask) != 0) {
|
||||
throw QuicTransportException(
|
||||
"Decoded delay overflows",
|
||||
quic::TransportErrorCode::FRAME_ENCODING_ERROR,
|
||||
frameType);
|
||||
}
|
||||
uint64_t adjustedDelay = delay->first << exponentToUse;
|
||||
uint64_t adjustedDelay = delay << exponentToUse;
|
||||
if (adjustedDelay >
|
||||
static_cast<uint64_t>(
|
||||
std::numeric_limits<std::chrono::microseconds::rep>::max())) {
|
||||
throw QuicTransportException(
|
||||
"Bad delay", quic::TransportErrorCode::FRAME_ENCODING_ERROR, frameType);
|
||||
} else if (UNLIKELY(adjustedDelay > 1000 * 1000 * 1000 /* 1000s */)) {
|
||||
LOG(ERROR) << "Quic recvd long ack delay=" << adjustedDelay;
|
||||
adjustedDelay = 0;
|
||||
}
|
||||
return adjustedDelay;
|
||||
}
|
||||
@@ -220,8 +217,17 @@ ReadAckFrame decodeAckFrame(
|
||||
PacketNum currentPacketNum =
|
||||
nextAckedPacketLen(largestAcked, firstAckBlockLen->first);
|
||||
frame.largestAcked = largestAcked;
|
||||
frame.ackDelay = std::chrono::microseconds(
|
||||
computeAdjustedDelay(frameType, ackDelayExponentToUse, ackDelay));
|
||||
|
||||
auto adjustedDelay = convertEncodedDurationToMicroseconds(
|
||||
frameType, ackDelayExponentToUse, ackDelay->first);
|
||||
|
||||
if (UNLIKELY(adjustedDelay > 1000 * 1000 * 1000 /* 1000s */)) {
|
||||
LOG(ERROR) << "Quic recvd long ack delay=" << adjustedDelay
|
||||
<< " frame type: " << static_cast<uint64_t>(frameType);
|
||||
adjustedDelay = 0;
|
||||
}
|
||||
frame.ackDelay = std::chrono::microseconds(adjustedDelay);
|
||||
|
||||
frame.ackBlocks.emplace_back(currentPacketNum, largestAcked);
|
||||
for (uint64_t numBlocks = 0; numBlocks < additionalAckBlocks->first;
|
||||
++numBlocks) {
|
||||
@@ -318,8 +324,8 @@ ReadAckFrame decodeAckFrameWithReceivedTimestamps(
|
||||
quic::FrameType::ACK_RECEIVE_TIMESTAMPS);
|
||||
}
|
||||
DCHECK_LT(receiveTimestampsExponentToUse, sizeof(delta->first) * 8);
|
||||
auto adjustedDelta = computeAdjustedDelay(
|
||||
frameType, receiveTimestampsExponentToUse, delta);
|
||||
auto adjustedDelta = convertEncodedDurationToMicroseconds(
|
||||
frameType, receiveTimestampsExponentToUse, delta->first);
|
||||
timeStampRange.deltas.push_back(adjustedDelta);
|
||||
}
|
||||
frame.recvdPacketsTimestampRanges.emplace_back(timeStampRange);
|
||||
|
||||
Reference in New Issue
Block a user