gfe-relnote: Deprecate gfe2_reloadable_flag_quic_detect_spurious_loss.
PiperOrigin-RevId: 276694630
Change-Id: I76f4f8cb9098fdb63cb5f9e3d68c11e97546c1ab
diff --git a/quic/core/congestion_control/general_loss_algorithm.cc b/quic/core/congestion_control/general_loss_algorithm.cc
index 3502031..5f7115a 100644
--- a/quic/core/congestion_control/general_loss_algorithm.cc
+++ b/quic/core/congestion_control/general_loss_algorithm.cc
@@ -39,7 +39,6 @@
void GeneralLossAlgorithm::SetLossDetectionType(LossDetectionType loss_type) {
loss_detection_timeout_ = QuicTime::Zero();
- largest_sent_on_spurious_retransmit_.Clear();
loss_type_ = loss_type;
if (loss_type == kAdaptiveTime) {
reordering_shift_ = kDefaultAdaptiveLossDelayShift;
@@ -185,36 +184,6 @@
return loss_detection_timeout_;
}
-void GeneralLossAlgorithm::SpuriousRetransmitDetected(
- const QuicUnackedPacketMap& unacked_packets,
- QuicTime time,
- const RttStats& rtt_stats,
- QuicPacketNumber spurious_retransmission) {
- if (loss_type_ != kAdaptiveTime || reordering_shift_ == 0) {
- return;
- }
- // Calculate the extra time needed so this wouldn't have been declared lost.
- // Extra time needed is based on how long it's been since the spurious
- // retransmission was sent, because the SRTT and latest RTT may have changed.
- QuicTime::Delta extra_time_needed =
- time -
- unacked_packets.GetTransmissionInfo(spurious_retransmission).sent_time;
- // Increase the reordering fraction until enough time would be allowed.
- QuicTime::Delta max_rtt =
- std::max(rtt_stats.previous_srtt(), rtt_stats.latest_rtt());
-
- if (largest_sent_on_spurious_retransmit_.IsInitialized() &&
- spurious_retransmission <= largest_sent_on_spurious_retransmit_) {
- return;
- }
- largest_sent_on_spurious_retransmit_ = unacked_packets.largest_sent_packet();
- QuicTime::Delta proposed_extra_time(QuicTime::Delta::Zero());
- do {
- proposed_extra_time = max_rtt >> reordering_shift_;
- --reordering_shift_;
- } while (proposed_extra_time < extra_time_needed && reordering_shift_ > 0);
-}
-
void GeneralLossAlgorithm::SpuriousLossDetected(
const QuicUnackedPacketMap& unacked_packets,
const RttStats& rtt_stats,
diff --git a/quic/core/congestion_control/general_loss_algorithm.h b/quic/core/congestion_control/general_loss_algorithm.h
index c9da837..d501cc6 100644
--- a/quic/core/congestion_control/general_loss_algorithm.h
+++ b/quic/core/congestion_control/general_loss_algorithm.h
@@ -47,13 +47,6 @@
// Returns a non-zero value when the early retransmit timer is active.
QuicTime GetLossTimeout() const override;
- // Increases the loss detection threshold for time loss detection.
- void SpuriousRetransmitDetected(
- const QuicUnackedPacketMap& unacked_packets,
- QuicTime time,
- const RttStats& rtt_stats,
- QuicPacketNumber spurious_retransmission) override;
-
// Called to increases time and/or packet threshold.
void SpuriousLossDetected(const QuicUnackedPacketMap& unacked_packets,
const RttStats& rtt_stats,
@@ -79,9 +72,6 @@
private:
QuicTime loss_detection_timeout_;
- // Largest sent packet when a spurious retransmit is detected.
- // Prevents increasing the reordering threshold multiple times per epoch.
- QuicPacketNumber largest_sent_on_spurious_retransmit_;
LossDetectionType loss_type_;
// Fraction of a max(SRTT, latest_rtt) to permit reordering before declaring
// loss. Fraction calculated by shifting max(SRTT, latest_rtt) to the right
diff --git a/quic/core/congestion_control/general_loss_algorithm_test.cc b/quic/core/congestion_control/general_loss_algorithm_test.cc
index f1ba35a..0c41f65 100644
--- a/quic/core/congestion_control/general_loss_algorithm_test.cc
+++ b/quic/core/congestion_control/general_loss_algorithm_test.cc
@@ -514,21 +514,9 @@
// Advance the time 1/4 RTT and indicate the loss was spurious.
// The new threshold should be 1/2 RTT.
clock_.AdvanceTime(rtt_stats_.smoothed_rtt() * (1.0f / 4));
- if (GetQuicReloadableFlag(quic_detect_spurious_loss)) {
- loss_algorithm_.SpuriousLossDetected(unacked_packets_, rtt_stats_,
- clock_.Now(), QuicPacketNumber(1),
- QuicPacketNumber(2));
- EXPECT_EQ(1, loss_algorithm_.reordering_shift());
- return;
- }
- loss_algorithm_.SpuriousRetransmitDetected(unacked_packets_, clock_.Now(),
- rtt_stats_, QuicPacketNumber(11));
- EXPECT_EQ(1, loss_algorithm_.reordering_shift());
-
- // Detect another spurious retransmit and ensure the threshold doesn't
- // increase again.
- loss_algorithm_.SpuriousRetransmitDetected(unacked_packets_, clock_.Now(),
- rtt_stats_, QuicPacketNumber(12));
+ loss_algorithm_.SpuriousLossDetected(unacked_packets_, rtt_stats_,
+ clock_.Now(), QuicPacketNumber(1),
+ QuicPacketNumber(2));
EXPECT_EQ(1, loss_algorithm_.reordering_shift());
}
diff --git a/quic/core/congestion_control/loss_detection_interface.h b/quic/core/congestion_control/loss_detection_interface.h
index f686785..af9c196 100644
--- a/quic/core/congestion_control/loss_detection_interface.h
+++ b/quic/core/congestion_control/loss_detection_interface.h
@@ -35,16 +35,6 @@
// Returns QuicTime::Zero if no alarm needs to be set.
virtual QuicTime GetLossTimeout() const = 0;
- // Called when a |spurious_retransmission| is detected. The original
- // transmission must have been caused by DetectLosses.
- // TODO(fayang): Remove this method when deprecating
- // quic_detect_spurious_loss.
- virtual void SpuriousRetransmitDetected(
- const QuicUnackedPacketMap& unacked_packets,
- QuicTime time,
- const RttStats& rtt_stats,
- QuicPacketNumber spurious_retransmission) = 0;
-
// Called when |packet_number| was detected lost but gets acked later.
virtual void SpuriousLossDetected(
const QuicUnackedPacketMap& unacked_packets,
diff --git a/quic/core/congestion_control/uber_loss_algorithm.cc b/quic/core/congestion_control/uber_loss_algorithm.cc
index 3b669e2..1aa10ff 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.cc
+++ b/quic/core/congestion_control/uber_loss_algorithm.cc
@@ -70,17 +70,6 @@
return loss_timeout;
}
-void UberLossAlgorithm::SpuriousRetransmitDetected(
- const QuicUnackedPacketMap& unacked_packets,
- QuicTime time,
- const RttStats& rtt_stats,
- QuicPacketNumber spurious_retransmission) {
- general_loss_algorithms_[unacked_packets.GetPacketNumberSpace(
- spurious_retransmission)]
- .SpuriousRetransmitDetected(unacked_packets, time, rtt_stats,
- spurious_retransmission);
-}
-
void UberLossAlgorithm::SpuriousLossDetected(
const QuicUnackedPacketMap& unacked_packets,
const RttStats& rtt_stats,
diff --git a/quic/core/congestion_control/uber_loss_algorithm.h b/quic/core/congestion_control/uber_loss_algorithm.h
index 0d2b788..a97fe08 100644
--- a/quic/core/congestion_control/uber_loss_algorithm.h
+++ b/quic/core/congestion_control/uber_loss_algorithm.h
@@ -41,13 +41,6 @@
// Returns the earliest time the early retransmit timer should be active.
QuicTime GetLossTimeout() const override;
- // Increases the loss detection threshold for time loss detection.
- void SpuriousRetransmitDetected(
- const QuicUnackedPacketMap& unacked_packets,
- QuicTime time,
- const RttStats& rtt_stats,
- QuicPacketNumber spurious_retransmission) override;
-
// Called to increases time or packet threshold.
void SpuriousLossDetected(const QuicUnackedPacketMap& unacked_packets,
const RttStats& rtt_stats,
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 13ac894..852f9d2 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -103,7 +103,6 @@
consecutive_pto_count_(0),
handshake_mode_disabled_(false),
forward_secure_packet_acked_(false),
- detect_spurious_losses_(GetQuicReloadableFlag(quic_detect_spurious_loss)),
neuter_handshake_packets_once_(
GetQuicReloadableFlag(quic_neuter_handshake_packets_once)) {
SetSendAlgorithm(congestion_control_type);
@@ -253,18 +252,16 @@
uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection);
uber_loss_algorithm_.SetReorderingShift(kDefaultLossDelayShift);
}
- if (GetQuicReloadableFlag(quic_detect_spurious_loss)) {
- if (config.HasClientRequestedIndependentOption(kILD2, perspective)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_ietf_loss_detection, 3, 4);
- uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection);
- uber_loss_algorithm_.EnableAdaptiveReorderingThreshold();
- }
- if (config.HasClientRequestedIndependentOption(kILD3, perspective)) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_ietf_loss_detection, 4, 4);
- uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection);
- uber_loss_algorithm_.SetReorderingShift(kDefaultLossDelayShift);
- uber_loss_algorithm_.EnableAdaptiveReorderingThreshold();
- }
+ if (config.HasClientRequestedIndependentOption(kILD2, perspective)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_ietf_loss_detection, 3, 4);
+ uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection);
+ uber_loss_algorithm_.EnableAdaptiveReorderingThreshold();
+ }
+ if (config.HasClientRequestedIndependentOption(kILD3, perspective)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_ietf_loss_detection, 4, 4);
+ uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection);
+ uber_loss_algorithm_.SetReorderingShift(kDefaultLossDelayShift);
+ uber_loss_algorithm_.EnableAdaptiveReorderingThreshold();
}
}
if (config.HasClientSentConnectionOption(kCONH, perspective)) {
@@ -515,18 +512,6 @@
}
}
-void QuicSentPacketManager::RecordSpuriousRetransmissions(
- const QuicTransmissionInfo& info,
- QuicPacketNumber acked_packet_number) {
- RecordOneSpuriousRetransmission(info);
- if (!detect_spurious_losses_ &&
- info.transmission_type == LOSS_RETRANSMISSION) {
- // Only inform the loss detection of spurious retransmits it caused.
- loss_algorithm_->SpuriousRetransmitDetected(
- unacked_packets_, clock_->Now(), rtt_stats_, acked_packet_number);
- }
-}
-
void QuicSentPacketManager::MarkPacketHandled(QuicPacketNumber packet_number,
QuicTransmissionInfo* info,
QuicTime ack_receive_time,
@@ -547,13 +532,12 @@
QUIC_DVLOG(1) << "Detect spurious retransmitted packet " << packet_number
<< " transmission type: "
<< TransmissionTypeToString(info->transmission_type);
- RecordSpuriousRetransmissions(*info, packet_number);
+ RecordOneSpuriousRetransmission(*info);
}
}
- if (detect_spurious_losses_ && info->state == LOST) {
+ if (info->state == LOST) {
// Record as a spurious loss as a packet previously declared lost gets
// acked.
- QUIC_RELOADABLE_FLAG_COUNT(quic_detect_spurious_loss);
const PacketNumberSpace packet_number_space =
unacked_packets_.GetPacketNumberSpace(info->encryption_level);
const QuicPacketNumber previous_largest_acked =
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index a517dec..e6a5b6f 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -491,11 +491,6 @@
// this function.
void RecordOneSpuriousRetransmission(const QuicTransmissionInfo& info);
- // Notify observers about spurious retransmits of packet with
- // QuicTransmissionInfo |info|.
- void RecordSpuriousRetransmissions(const QuicTransmissionInfo& info,
- QuicPacketNumber acked_packet_number);
-
// Sets the initial RTT of the connection.
void SetInitialRtt(QuicTime::Delta rtt);
@@ -626,9 +621,6 @@
// ENCRYPTION_FORWARD_SECURE packet.
bool forward_secure_packet_acked_;
- // Latched value of quic_detect_spurious_loss.
- const bool detect_spurious_losses_;
-
// Latched value of quic_neuter_handshake_packets_once.
const bool neuter_handshake_packets_once_;
};
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 7872f74..e86d950 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -663,14 +663,9 @@
uint64_t acked[] = {3};
ExpectAcksAndLosses(false, acked, QUIC_ARRAYSIZE(acked), nullptr, 0);
EXPECT_CALL(*loss_algorithm, DetectLosses(_, _, _, _, _, _));
- if (GetQuicReloadableFlag(quic_detect_spurious_loss)) {
- EXPECT_CALL(*loss_algorithm,
- SpuriousLossDetected(_, _, _, QuicPacketNumber(3),
- QuicPacketNumber(4)));
- } else {
- EXPECT_CALL(*loss_algorithm,
- SpuriousRetransmitDetected(_, _, _, QuicPacketNumber(5)));
- }
+ EXPECT_CALL(*loss_algorithm,
+ SpuriousLossDetected(_, _, _, QuicPacketNumber(3),
+ QuicPacketNumber(4)));
manager_.OnAckFrameStart(QuicPacketNumber(4), QuicTime::Delta::Infinite(),
clock_.Now());
manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(5));
@@ -1810,7 +1805,6 @@
TEST_F(QuicSentPacketManagerTest,
NegotiateIetfLossDetectionAdaptiveReorderingThreshold) {
SetQuicReloadableFlag(quic_enable_ietf_loss_detection, true);
- SetQuicReloadableFlag(quic_detect_spurious_loss, true);
EXPECT_EQ(kNack, QuicSentPacketManagerPeer::GetLossAlgorithm(&manager_)
->GetLossDetectionType());
EXPECT_FALSE(
@@ -1835,7 +1829,6 @@
TEST_F(QuicSentPacketManagerTest,
NegotiateIetfLossDetectionAdaptiveReorderingThreshold2) {
SetQuicReloadableFlag(quic_enable_ietf_loss_detection, true);
- SetQuicReloadableFlag(quic_detect_spurious_loss, true);
EXPECT_EQ(kNack, QuicSentPacketManagerPeer::GetLossAlgorithm(&manager_)
->GetLossDetectionType());
EXPECT_FALSE(
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index 9f8f23c..e39b416 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -966,11 +966,6 @@
const AckedPacketVector& packets_acked,
LostPacketVector* packets_lost));
MOCK_CONST_METHOD0(GetLossTimeout, QuicTime());
- MOCK_METHOD4(SpuriousRetransmitDetected,
- void(const QuicUnackedPacketMap&,
- QuicTime,
- const RttStats&,
- QuicPacketNumber));
MOCK_METHOD5(SpuriousLossDetected,
void(const QuicUnackedPacketMap&,
const RttStats&,
