catchunks: avoid excess window decrease in certain conditions

Refs: #5202
Change-Id: I7eeed18fbdfc3ef7a5a277da34f9de0c7068a61d

tests/chunks/pipeline-interests-aimd.t.cpp

@@ -72,8 +72,6 @@ protected:
   static constexpr double MARGIN = 0.001;
 };
 
-constexpr double PipelineInterestAimdFixture::MARGIN;
-
 BOOST_AUTO_TEST_SUITE(Chunks)
 BOOST_FIXTURE_TEST_SUITE(TestPipelineInterestsAimd, PipelineInterestAimdFixture)
 
@@ -190,9 +188,7 @@ BOOST_AUTO_TEST_CASE(Timeout)
 
   BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, 2);
   BOOST_CHECK_EQUAL(pipeline->m_nRetransmitted, 2);
-  BOOST_CHECK_EQUAL(pipeline->m_nTimeouts,
-      pipeline->m_nRetransmitted + pipeline->m_nSkippedRetx);
-
+  BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, pipeline->m_nRetransmitted + pipeline->m_nSkippedRetx);
 }
 
 BOOST_AUTO_TEST_CASE(CongestionMarksWithCwa)
@@ -545,6 +541,58 @@ BOOST_AUTO_TEST_CASE(SegmentInfoMaintenance)
   BOOST_CHECK_EQUAL(rttEstimator.getEstimatedRto(), prevRto);
 }
 
+BOOST_AUTO_TEST_CASE(Bug5202)
+{
+  // If an interest is pending during a window decrease, it should not trigger
+  // another window decrease when it times out.
+  // This test emulates a network where RTT = 20ms and transmission time = 1ms.
+
+  // adding small sample to RTT estimator. This should set rto = 200ms
+  rttEstimator.addMeasurement(time::nanoseconds(1));
+  BOOST_REQUIRE_EQUAL(rttEstimator.getEstimatedRto(), time::milliseconds(200));
+
+  nDataSegments = 300;
+  pipeline->m_ssthresh = 0;
+  pipeline->m_cwnd = 20;
+
+  run(name);
+
+  advanceClocks(time::nanoseconds(1));
+  BOOST_CHECK_EQUAL(face.sentInterests.size(), 20);
+  advanceClocks(time::milliseconds(20));
+
+  // Segment 1 is lost. Receive segment 0 and 2-99
+  face.receive(*makeDataWithSegment(0));
+  advanceClocks(time::milliseconds(1));
+  for (uint64_t i = 2; i <= 99; ++i) {
+    face.receive(*makeDataWithSegment(i));
+    advanceClocks(time::milliseconds(1));
+  }
+
+  // Segment 100 is lost. Receive segment 100 to 181
+  for (uint64_t i = 101; i <= 181; ++i) {
+    face.receive(*makeDataWithSegment(i));
+    advanceClocks(time::milliseconds(1));
+  }
+
+  // 200ms passed after sending segment 1, check for timeout
+  BOOST_CHECK_GT(pipeline->m_cwnd, 27./2);
+  BOOST_CHECK_LT(pipeline->m_cwnd, 28./2);
+  BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, 1);
+  BOOST_CHECK_EQUAL(pipeline->m_nLossDecr, 1);
+
+  // Receive segment 182 to 300
+  for (uint64_t i = 182; i <= 300; ++i) {
+    face.receive(*makeDataWithSegment(i));
+    advanceClocks(time::milliseconds(1));
+  }
+
+  // The second packet should timeout now
+  BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, 2);
+  // This timeout should NOT trigger another window decrease
+  BOOST_CHECK_EQUAL(pipeline->m_nLossDecr, 1);
+}
+
 BOOST_AUTO_TEST_CASE(PrintSummaryWithNoRttMeasurements)
 {
   // test the console ouptut when no RTT measurement is available,
@@ -589,7 +637,6 @@ BOOST_AUTO_TEST_CASE(StopsWhenFileSizeLessThanChunkSize)
   BOOST_CHECK_EQUAL(face.getNPendingInterests(), 0);
 }
 
-
 BOOST_AUTO_TEST_SUITE_END() // TestPipelineInterestsAimd
 BOOST_AUTO_TEST_SUITE_END() // Chunks
 

tests/chunks/pipeline-interests-cubic.t.cpp

@@ -72,8 +72,6 @@ protected:
   static constexpr double MARGIN = 0.001;
 };
 
-constexpr double PipelineInterestCubicFixture::MARGIN;
-
 BOOST_AUTO_TEST_SUITE(Chunks)
 BOOST_FIXTURE_TEST_SUITE(TestPipelineInterestsCubic, PipelineInterestCubicFixture)
 

tools/chunks/catchunks/pipeline-interests-adaptive.cpp

@@ -34,8 +34,6 @@
 
 namespace ndn::chunks {
 
-constexpr double PipelineInterestsAdaptive::MIN_SSTHRESH;
-
 PipelineInterestsAdaptive::PipelineInterestsAdaptive(Face& face,
                                                      RttEstimatorWithStats& rttEstimator,
                                                      const Options& opts)
@@ -83,6 +81,7 @@ PipelineInterestsAdaptive::checkRto()
     return;
 
   bool hasTimeout = false;
+  uint64_t highTimeoutSeg = 0;
 
   for (auto& entry : m_segmentInfo) {
     SegmentInfo& segInfo = entry.second;
@@ -91,13 +90,14 @@ PipelineInterestsAdaptive::checkRto()
       if (timeElapsed > segInfo.rto) { // timer expired?
         m_nTimeouts++;
         hasTimeout = true;
+        highTimeoutSeg = std::max(highTimeoutSeg, entry.first);
         enqueueForRetransmission(entry.first);
       }
     }
   }
 
   if (hasTimeout) {
-    recordTimeout();
+    recordTimeout(highTimeoutSeg);
     schedulePackets();
   }
 
@@ -227,9 +227,7 @@ PipelineInterestsAdaptive::handleData(const Interest& interest, const Data& data
               << ", rto=" << segInfo.rto.count() / 1e6 << "ms\n";
   }
 
-  if (m_highData < recvSegNo) {
-    m_highData = recvSegNo;
-  }
+  m_highData = std::max(m_highData, recvSegNo);
 
   // for segments in retx queue, we must not decrement m_nInFlight
   // because it was already decremented when the segment timed out
@@ -310,7 +308,7 @@ PipelineInterestsAdaptive::handleNack(const Interest& interest, const lp::Nack&
     case lp::NackReason::CONGESTION:
       // treated the same as timeout for now
       enqueueForRetransmission(segNo);
-      recordTimeout();
+      recordTimeout(segNo);
       schedulePackets();
       break;
     default:
@@ -327,16 +325,19 @@ PipelineInterestsAdaptive::handleLifetimeExpiration(const Interest& interest)
     return;
 
   m_nTimeouts++;
-  enqueueForRetransmission(getSegmentFromPacket(interest));
-  recordTimeout();
+
+  uint64_t segNo = getSegmentFromPacket(interest);
+  enqueueForRetransmission(segNo);
+  recordTimeout(segNo);
   schedulePackets();
 }
 
 void
-PipelineInterestsAdaptive::recordTimeout()
+PipelineInterestsAdaptive::recordTimeout(uint64_t segNo)
 {
-  if (m_options.disableCwa || m_highData > m_recPoint) {
-    // react to only one timeout per RTT (conservative window adaptation)
+  if (m_options.disableCwa || segNo > m_recPoint) {
+    // interests that are still outstanding during a timeout event
+    // should not trigger another window decrease later (bug #5202)
     m_recPoint = m_highInterest;
 
     decreaseWindow();

tools/chunks/catchunks/pipeline-interests-adaptive.hpp

@@ -172,7 +172,7 @@ private:
   handleLifetimeExpiration(const Interest& interest);
 
   void
-  recordTimeout();
+  recordTimeout(uint64_t segNo);
 
   void
   enqueueForRetransmission(uint64_t segNo);