Skip to content
Permalink
Browse files

Separate app-limited and app-idle

Summary:
Current app-limited is defined as the connection doesn't have a no
non-control stream. I'm changing this to be app-idle. And app-limited will be
changed to a state in connection where app isn't sending bytes fast enough to
keep congestion controller probing higher bandwidth.

Reviewed By: mjoras

Differential Revision: D15456199

fbshipit-source-id: e084bc133284ae37ee6da8ebb3c12f505011521e
  • Loading branch information...
yangchi authored and facebook-github-bot committed May 24, 2019
1 parent 41a6b82 commit 1dbe39fd76191010f0f2c0da6c9ffba30b6b2d36
@@ -2127,6 +2127,17 @@ void QuicTransportBase::writeSocketData() {
setIdleTimer();
conn_->receivedNewPacketBeforeWrite = false;
}
// Check if we are app-limited after finish this round of sending
auto currentSendBufLen = conn_->flowControlState.sumCurStreamBufferLen;
auto lossBufferEmpty = !conn_->streamManager->hasLoss() &&
conn_->cryptoState->initialStream.lossBuffer.empty() &&
conn_->cryptoState->handshakeStream.lossBuffer.empty() &&
conn_->cryptoState->oneRttStream.lossBuffer.empty();
if (conn_->congestionController &&
currentSendBufLen < conn_->udpSendPacketLen && lossBufferEmpty &&
conn_->congestionController->getWritableBytes()) {
conn_->congestionController->setAppLimited();
}
}
}
// Writing data could write out an ack which could cause us to cancel
@@ -366,7 +366,8 @@ void updateConnection(
packetNum,
(uint64_t)encodedSize,
(int)isHandshake,
(int)pureAck);
(int)pureAck,
pkt.isAppLimited);
conn.lossState.largestSent = std::max(conn.lossState.largestSent, packetNum);
if (conn.congestionController && !pureAck) {
conn.congestionController->onPacketSent(pkt);
@@ -1467,26 +1467,26 @@ INSTANTIATE_TEST_CASE_P(
QuicTransportImplTestUniBidi,
Values(true, false));

TEST_P(QuicTransportImplTestUniBidi, AppLimitedTest) {
TEST_P(QuicTransportImplTestUniBidi, AppIdleTest) {
auto& conn = transport->getConnectionState();
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);

EXPECT_CALL(*rawCongestionController, setAppLimited(false, _)).Times(0);
EXPECT_CALL(*rawCongestionController, setAppIdle(false, _)).Times(0);
auto stream = createStream(transport, GetParam());

EXPECT_CALL(*rawCongestionController, setAppLimited(true, _));
EXPECT_CALL(*rawCongestionController, setAppIdle(true, _));
transport->closeStream(stream);
}

TEST_P(QuicTransportImplTestUniBidi, AppLimitedTestControlStreams) {
TEST_P(QuicTransportImplTestUniBidi, AppIdleTestControlStreams) {
auto& conn = transport->getConnectionState();
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);

EXPECT_CALL(*rawCongestionController, setAppLimited(false, _)).Times(0);
EXPECT_CALL(*rawCongestionController, setAppIdle(false, _)).Times(0);
auto stream = createStream(transport, GetParam());
ASSERT_TRUE(stream);

@@ -1497,25 +1497,25 @@ TEST_P(QuicTransportImplTestUniBidi, AppLimitedTestControlStreams) {
ASSERT_TRUE(ctrlStream2);
transport->setControlStream(ctrlStream2);

EXPECT_CALL(*rawCongestionController, setAppLimited(true, _));
EXPECT_CALL(*rawCongestionController, setAppIdle(true, _));
transport->closeStream(stream);
}

TEST_P(QuicTransportImplTestUniBidi, AppLimitedTestOnlyControlStreams) {
TEST_P(QuicTransportImplTestUniBidi, AppIdleTestOnlyControlStreams) {
auto& conn = transport->getConnectionState();
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);

auto ctrlStream1 = createStream(transport, GetParam());
EXPECT_CALL(*rawCongestionController, setAppLimited(true, _)).Times(1);
EXPECT_CALL(*rawCongestionController, setAppIdle(true, _)).Times(1);
transport->setControlStream(ctrlStream1);
EXPECT_CALL(*rawCongestionController, setAppLimited(false, _)).Times(1);
EXPECT_CALL(*rawCongestionController, setAppIdle(false, _)).Times(1);
auto ctrlStream2 = createStream(transport, GetParam());
EXPECT_CALL(*rawCongestionController, setAppLimited(true, _)).Times(1);
EXPECT_CALL(*rawCongestionController, setAppIdle(true, _)).Times(1);
transport->setControlStream(ctrlStream2);

EXPECT_CALL(*rawCongestionController, setAppLimited(_, _)).Times(0);
EXPECT_CALL(*rawCongestionController, setAppIdle(_, _)).Times(0);
transport->closeStream(ctrlStream1);
transport->closeStream(ctrlStream2);
}
@@ -358,8 +358,96 @@ TEST_F(QuicTransportTest, WriteDataWithProbing) {
loopForWrites();
// Pending numProbePackets is cleared:
EXPECT_EQ(0, conn.pendingEvents.numProbePackets);
// both write and onPacketSent will inquire getWritableBytes
EXPECT_EQ(onPacketSentCounter + socketWriteCounter, getWritableBytesCounter);
transport_->close(folly::none);
}

TEST_F(QuicTransportTest, NotAppLimitedWithLoss) {
auto& conn = transport_->getConnectionState();
// Replace with MockConnectionCallback:
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, getWritableBytes())
.WillRepeatedly(Return(5000));

auto stream = transport_->createBidirectionalStream().value();
auto lossStream = transport_->createBidirectionalStream().value();
conn.streamManager->addLoss(lossStream);
conn.streamManager->getStream(lossStream)
->lossBuffer.emplace_back(
IOBuf::copyBuffer("Mountains may depart"), 0, false);
transport_->writeChain(
stream,
IOBuf::copyBuffer("An elephant sitting still"),
false,
false,
nullptr);
EXPECT_CALL(*rawCongestionController, setAppLimited()).Times(0);
loopForWrites();
transport_->close(folly::none);
}

TEST_F(QuicTransportTest, NotAppLimitedWithNoWritableBytes) {
auto& conn = transport_->getConnectionState();
// Replace with MockConnectionCallback:
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, getWritableBytes())
.WillRepeatedly(Invoke([&]() {
if (conn.outstandingPackets.empty()) {
return 5000;
}
return 0;
}));

auto stream = transport_->createBidirectionalStream().value();
transport_->writeChain(
stream,
IOBuf::copyBuffer("An elephant sitting still"),
false,
false,
nullptr);
EXPECT_CALL(*rawCongestionController, setAppLimited()).Times(0);
loopForWrites();
transport_->close(folly::none);
}

TEST_F(QuicTransportTest, NotAppLimitedWithLargeBuffer) {
auto& conn = transport_->getConnectionState();
// Replace with MockConnectionCallback:
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, getWritableBytes())
.WillRepeatedly(Return(5000));

auto stream = transport_->createBidirectionalStream().value();
auto buf = buildRandomInputData(100 * 2000);
transport_->writeChain(stream, buf->clone(), false, false, nullptr);
EXPECT_CALL(*rawCongestionController, setAppLimited()).Times(0);
loopForWrites();
transport_->close(folly::none);
}

TEST_F(QuicTransportTest, AppLimited) {
auto& conn = transport_->getConnectionState();
// Replace with MockConnectionCallback:
auto mockCongestionController = std::make_unique<MockCongestionController>();
auto rawCongestionController = mockCongestionController.get();
conn.congestionController = std::move(mockCongestionController);
EXPECT_CALL(*rawCongestionController, getWritableBytes())
.WillRepeatedly(Return(5000));

auto stream = transport_->createBidirectionalStream().value();
transport_->writeChain(
stream,
IOBuf::copyBuffer("An elephant sitting still"),
false,
false,
nullptr);
EXPECT_CALL(*rawCongestionController, setAppLimited()).Times(1);
loopForWrites();
transport_->close(folly::none);
}

@@ -308,7 +308,10 @@ std::chrono::microseconds Copa::getPacingInterval() const noexcept {

void Copa::setMinimalPacingInterval(std::chrono::microseconds) noexcept {}

void Copa::setAppLimited(bool, TimePoint) noexcept { /* unsupported */
void Copa::setAppIdle(bool, TimePoint) noexcept { /* unsupported */
}

void Copa::setAppLimited() { /* unsupported */
}

bool Copa::isAppLimited() const noexcept {
@@ -43,7 +43,8 @@ class Copa : public CongestionController {
uint64_t getBytesInFlight() const noexcept;

void setConnectionEmulation(uint8_t) noexcept override;
void setAppLimited(bool, TimePoint) noexcept override;
void setAppIdle(bool, TimePoint) noexcept override;
void setAppLimited() override;

// pacing related functions, not implemented for copa
bool canBePaced() const noexcept override;
@@ -158,7 +158,10 @@ std::chrono::microseconds NewReno::getPacingInterval() const noexcept {

void NewReno::setMinimalPacingInterval(std::chrono::microseconds) noexcept {}

void NewReno::setAppLimited(bool, TimePoint) noexcept { /* unsupported */
void NewReno::setAppIdle(bool, TimePoint) noexcept { /* unsupported */
}

void NewReno::setAppLimited() { /* unsupported */
}

bool NewReno::isAppLimited() const noexcept {
@@ -27,7 +27,9 @@ class NewReno : public CongestionController {
uint64_t getCongestionWindow() const noexcept override;
void setConnectionEmulation(uint8_t) noexcept override;
bool canBePaced() const noexcept override;
void setAppLimited(bool, TimePoint) noexcept override;
void setAppIdle(bool, TimePoint) noexcept override;
void setAppLimited() override;

CongestionControlType type() const noexcept override;

bool inSlowStart() const noexcept;
@@ -154,11 +154,11 @@ bool Cubic::canBePaced() const noexcept {
return pacingEnabled;
}

void Cubic::setAppLimited(bool limited, TimePoint eventTime) noexcept {
void Cubic::setAppIdle(bool idle, TimePoint eventTime) noexcept {
QUIC_TRACE(
cubic_applimited,
cubic_appidle,
conn_,
limited,
idle,
std::chrono::duration_cast<std::chrono::milliseconds>(
eventTime.time_since_epoch())
.count(),
@@ -167,22 +167,32 @@ void Cubic::setAppLimited(bool limited, TimePoint eventTime) noexcept {
steadyState_.lastReductionTime->time_since_epoch())
.count()
: -1);
bool currentAppLimited = isAppLimited();
if (!currentAppLimited && limited) {
bool currentAppIdle = isAppIdle();
if (!currentAppIdle && idle) {
quiescenceStart_ = eventTime;
}
if (!limited && currentAppLimited && *quiescenceStart_ <= eventTime &&
if (!idle && currentAppIdle && *quiescenceStart_ <= eventTime &&
steadyState_.lastReductionTime) {
*steadyState_.lastReductionTime +=
std::chrono::duration_cast<std::chrono::milliseconds>(
eventTime - *quiescenceStart_);
}
if (!limited) {
if (!idle) {
quiescenceStart_ = folly::none;
}
}

void Cubic::setAppLimited() {
// we use app-idle for Cubic
}

bool Cubic::isAppLimited() const noexcept {
// Or maybe always false. This doesn't really matter for Cubic. Channeling
// isAppIdle() makes testing easier.
return isAppIdle();
}

bool Cubic::isAppIdle() const noexcept {
return quiescenceStart_.hasValue();
}

@@ -97,7 +97,8 @@ class Cubic : public CongestionController {
uint64_t getCongestionWindow() const noexcept override;
bool canBePaced() const noexcept override;
void setConnectionEmulation(uint8_t) noexcept override;
void setAppLimited(bool limited, TimePoint eventTime) noexcept override;
void setAppIdle(bool idle, TimePoint eventTime) noexcept override;
void setAppLimited() override;

bool isAppLimited() const noexcept override;

@@ -116,6 +117,7 @@ class Cubic : public CongestionController {
CubicStates state_{CubicStates::Hystart};

private:
bool isAppIdle() const noexcept;
void onPacketAcked(const AckEvent& ack);
void onPacketAckedInHystart(const AckEvent& ack);
void onPacketAckedInSteady(const AckEvent& ack);
@@ -124,7 +124,7 @@ TEST_F(CubicTest, CwndIncreaseAfterReduction) {
EXPECT_EQ(CubicStates::Steady, cubic.state());
}

TEST_F(CubicTest, AppLimited) {
TEST_F(CubicTest, AppIdle) {
QuicConnectionStateBase conn(QuicNodeType::Client);
conn.udpSendPacketLen = 1500;
TestingCubic cubic(conn);
@@ -150,7 +150,7 @@ TEST_F(CubicTest, AppLimited) {
EXPECT_GT(cubic.getCongestionWindow(), cwnd);
cwnd = cubic.getCongestionWindow();

cubic.setAppLimited(true, reductionTime + 1100ms);
cubic.setAppIdle(true, reductionTime + 1100ms);
EXPECT_TRUE(cubic.isAppLimited());
auto packet2 = makeTestingWritePacket(2, 1000, 3000);
cubic.onPacketSent(packet2);
@@ -159,7 +159,7 @@ TEST_F(CubicTest, AppLimited) {
EXPECT_EQ(cubic.getCongestionWindow(), cwnd);

// 1 seconds of quiescence
cubic.setAppLimited(false, reductionTime + 2100ms);
cubic.setAppIdle(false, reductionTime + 2100ms);
EXPECT_FALSE(cubic.isAppLimited());
auto packet3 = makeTestingWritePacket(3, 1000, 4000);
cubic.onPacketSent(packet3);

0 comments on commit 1dbe39f

Please sign in to comment.
You can’t perform that action at this time.