Control write loop time limit from knob.

Summary:
This has been hardcoded to SRTT/25 for a long time. However, especially when using DSR this might not be the most appropriate since it can start to get close to real world SRTTs.

Control it via a knob, and also add the behavior such that setting it to 0 effectively disables the time limit.

Reviewed By: jbeshay

Differential Revision: D46084438

fbshipit-source-id: 7dc619fdff1bd5c3f8654c4936200e0340ef94f2

quic/QuicConstants.h

@@ -168,7 +168,10 @@ BETTER_ENUM(
     FIRE_LOOP_EARLY = 0x10001,
     // Controls the timer tick used for pacing
     PACING_TIMER_TICK = 0x10002,
-    DEFAULT_STREAM_PRIORITY = 0x10003)
+    // Controls default stream priority
+    DEFAULT_STREAM_PRIORITY = 0x10003,
+    // Controls write loop time fraction in terms of srtt
+    WRITE_LOOP_TIME_FRACTION = 0x10004)
 
 enum class FrameType : uint64_t {
   PADDING = 0x00,

quic/api/QuicTransportFunctions.cpp

@@ -403,6 +403,7 @@ bool writeLoopTimeLimit(
     TimePoint loopBeginTime,
     const QuicConnectionStateBase& connection) {
   return connection.lossState.srtt == 0us ||
+      connection.transportSettings.writeLimitRttFraction == 0 ||
       Clock::now() - loopBeginTime < connection.lossState.srtt /
           connection.transportSettings.writeLimitRttFraction;
 }

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -4077,6 +4077,56 @@ TEST_F(QuicTransportFunctionsTest, WriteLimitBytRttFraction) {
       res.packetsWritten);
 }
 
+TEST_F(QuicTransportFunctionsTest, WriteLimitBytRttFractionNoLimit) {
+  auto conn = createConn();
+  conn->lossState.srtt = 50ms;
+  auto mockCongestionController =
+      std::make_unique<NiceMock<MockCongestionController>>();
+  auto rawCongestionController = mockCongestionController.get();
+  conn->congestionController = std::move(mockCongestionController);
+  conn->transportSettings.batchingMode = QuicBatchingMode::BATCHING_MODE_NONE;
+  conn->transportSettings.writeLimitRttFraction = 0;
+
+  EventBase evb;
+  auto socket =
+      std::make_unique<NiceMock<folly::test::MockAsyncUDPSocket>>(&evb);
+  auto rawSocket = socket.get();
+
+  auto stream1 = conn->streamManager->createNextBidirectionalStream().value();
+  auto buf = buildRandomInputData(2048 * 2048);
+  writeDataToQuicStream(*stream1, buf->clone(), true);
+
+  EXPECT_CALL(*rawSocket, write(_, _)).WillRepeatedly(Return(1));
+  EXPECT_CALL(*rawCongestionController, getWritableBytes())
+      .WillRepeatedly(Return(50));
+  auto writeLoopBeginTime = Clock::now();
+  auto res = writeQuicDataToSocket(
+      *rawSocket,
+      *conn,
+      *conn->clientConnectionId,
+      *conn->serverConnectionId,
+      *aead,
+      *headerCipher,
+      getVersion(*conn),
+      1000 /* packetLimit */,
+      writeLoopBeginTime);
+
+  EXPECT_GT(1000, res.packetsWritten);
+  EXPECT_EQ(res.probesWritten, 0);
+
+  res = writeQuicDataToSocket(
+      *rawSocket,
+      *conn,
+      *conn->clientConnectionId,
+      *conn->serverConnectionId,
+      *aead,
+      *headerCipher,
+      getVersion(*conn),
+      1000 /* packetLimit */,
+      writeLoopBeginTime);
+  EXPECT_EQ(1000, res.packetsWritten);
+}
+
 TEST_F(QuicTransportFunctionsTest, CongestionControlWritableBytesRoundUp) {
   auto conn = createConn();
   conn->udpSendPacketLen = 2000;

quic/dsr/frontend/test/WriteFunctionsTest.cpp

@@ -91,6 +91,34 @@ TEST_F(WriteFunctionsTest, WriteLoopTimeLimit) {
   EXPECT_TRUE(verifyAllOutstandingsAreDSR());
 }
 
+TEST_F(WriteFunctionsTest, WriteLoopTimeLimitNoLimit) {
+  prepareFlowControlAndStreamLimit();
+  auto streamId = prepareOneStream(3000);
+  auto cid = getTestConnectionId();
+  auto stream = conn_.streamManager->findStream(streamId);
+  auto currentBufMetaOffset = stream->writeBufMeta.offset;
+  size_t packetLimit = 2;
+  conn_.lossState.srtt = 100ms;
+  conn_.transportSettings.writeLimitRttFraction = 0;
+  EXPECT_EQ(2, writePacketizationRequest(conn_, cid, packetLimit, *aead_));
+  EXPECT_GT(stream->writeBufMeta.offset, currentBufMetaOffset);
+  EXPECT_EQ(2, stream->retransmissionBufMetas.size());
+  EXPECT_EQ(2, countInstructions(streamId));
+  EXPECT_EQ(2, conn_.outstandings.packets.size());
+  EXPECT_TRUE(verifyAllOutstandingsAreDSR());
+
+  // Fake the time so it's in the past.
+  auto writeLoopBeginTime = Clock::now() - 200ms;
+  EXPECT_EQ(
+      1,
+      writePacketizationRequest(
+          conn_, cid, packetLimit, *aead_, writeLoopBeginTime));
+  EXPECT_EQ(3, stream->retransmissionBufMetas.size());
+  EXPECT_EQ(3, countInstructions(streamId));
+  EXPECT_EQ(3, conn_.outstandings.packets.size());
+  EXPECT_TRUE(verifyAllOutstandingsAreDSR());
+}
+
 TEST_F(WriteFunctionsTest, WriteTwoInstructions) {
   prepareFlowControlAndStreamLimit();
   auto streamId = prepareOneStream(2000);

quic/server/QuicServerTransport.cpp

@@ -1071,6 +1071,15 @@ void QuicServerTransport::registerAllTransportKnobParamHandlers() {
             Priority(level, incremental);
         VLOG(3) << "DEFAULT_STREAM_PRIORITY KnobParam received: " << val;
       });
+  registerTransportKnobParamHandler(
+      static_cast<uint64_t>(TransportKnobParamId::WRITE_LOOP_TIME_FRACTION),
+      [](QuicServerTransport* serverTransport, TransportKnobParam::Val value) {
+        CHECK(serverTransport);
+        auto val = std::get<uint64_t>(value);
+        auto serverConn = serverTransport->serverConn_;
+        serverConn->transportSettings.writeLimitRttFraction = val;
+        VLOG(3) << "WRITE_LOOP_TIME_FRACTION KnobParam received: " << val;
+      });
 }
 
 QuicConnectionStats QuicServerTransport::getConnectionsStats() const {

quic/server/test/QuicServerTransportTest.cpp

@@ -4837,6 +4837,10 @@ TEST_F(QuicServerTransportTest, TestAckFrequencyPolicyKnobHandler) {
       {{static_cast<uint64_t>(TransportKnobParamId::DEFAULT_STREAM_PRIORITY),
         "4,0,10"}});
   EXPECT_EQ(server->getTransportSettings().defaultPriority, Priority(4, false));
+  server->handleKnobParams(
+      {{static_cast<uint64_t>(TransportKnobParamId::WRITE_LOOP_TIME_FRACTION),
+        uint64_t(2)}});
+  EXPECT_EQ(server->getTransportSettings().writeLimitRttFraction, 2);
 }
 
 TEST_F(QuicServerTransportTest, TestSetMaxPacingRateLifecycle) {