lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-07-30 14:43:05 +03:00
Code Activity

Back out "If there's lost data, we have data to write."

Browse Source 
Summary: This is causing empty write loops for some reason.

Reviewed By: jbeshay, lnicco

Differential Revision: D32990291

fbshipit-source-id: 2a183d591de54c7fe0ca54aea828a697a4cd9af0
This commit is contained in:
Matt Joras
2021-12-09 14:34:19 -08:00
committed by Facebook GitHub Bot
parent 1e7287fcdb
commit 5b9a9705a0
5 changed files with 5 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
quic/api/QuicPacketScheduler.cpp
View File

@ -473,9 +473,8 @@ void StreamFrameScheduler::writeStreams(PacketBuilderInterface& builder) {
} // namespace quic } // namespace quic
bool StreamFrameScheduler::hasPendingData() const { bool StreamFrameScheduler::hasPendingData() const {
return conn_.streamManager->hasLoss() || return conn_.streamManager->hasNonDSRWritable() &&
(conn_.streamManager->hasNonDSRWritable() && getSendConnFlowControlBytesWire(conn_) > 0;
getSendConnFlowControlBytesWire(conn_) > 0);
} }
bool StreamFrameScheduler::writeStreamFrame( bool StreamFrameScheduler::writeStreamFrame(

6
quic/api/QuicTransportFunctions.cpp
View File

@ -1601,10 +1601,8 @@ WriteDataReason hasNonAckDataToWrite(const QuicConnectionStateBase& conn) {
if (conn.streamManager->hasBlocked()) { if (conn.streamManager->hasBlocked()) {
return WriteDataReason::BLOCKED; return WriteDataReason::BLOCKED;
} }
// If we have lost data or flow control + stream data. if (getSendConnFlowControlBytesWire(conn) != 0 &&
if (conn.streamManager->hasLoss() || conn.streamManager->hasWritable()) {
(getSendConnFlowControlBytesWire(conn) != 0 &&
conn.streamManager->hasWritable())) {
return WriteDataReason::STREAM; return WriteDataReason::STREAM;
} }
if (!conn.pendingEvents.frames.empty()) { if (!conn.pendingEvents.frames.empty()) {

3
quic/api/test/QuicPacketSchedulerTest.cpp
View File

@ -1641,7 +1641,6 @@ TEST_F(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) {
conn.streamManager->updateWritableStreams(*stream); conn.streamManager->updateWritableStreams(*stream);
StreamFrameScheduler scheduler(conn); StreamFrameScheduler scheduler(conn);
EXPECT_TRUE(scheduler.hasPendingData());
ShortHeader shortHeader1( ShortHeader shortHeader1(
ProtectionType::KeyPhaseZero, ProtectionType::KeyPhaseZero,
getTestConnectionId(), getTestConnectionId(),
@ -1667,8 +1666,6 @@ TEST_F(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) {
stream->lossBuffer.emplace_back(std::move(*stream->retransmissionBuffer[0])); stream->lossBuffer.emplace_back(std::move(*stream->retransmissionBuffer[0]));
stream->retransmissionBuffer.clear(); stream->retransmissionBuffer.clear();
conn.streamManager->updateWritableStreams(*stream); conn.streamManager->updateWritableStreams(*stream);
conn.streamManager->updateLossStreams(*stream);
EXPECT_TRUE(scheduler.hasPendingData());
// Write again // Write again
ShortHeader shortHeader2( ShortHeader shortHeader2(

18
quic/api/test/QuicTransportFunctionsTest.cpp
View File

@ -2804,24 +2804,6 @@ TEST_F(QuicTransportFunctionsTest, ShouldWriteDataNoConnFlowControl) {
EXPECT_EQ(WriteDataReason::NO_WRITE, shouldWriteData(*conn)); EXPECT_EQ(WriteDataReason::NO_WRITE, shouldWriteData(*conn));
} }
TEST_F(QuicTransportFunctionsTest, ShouldWriteDataNoConnFlowControlLoss) {
auto conn = createConn();
conn->oneRttWriteCipher = test::createNoOpAead();
auto mockCongestionController =
std::make_unique<NiceMock<MockCongestionController>>();
auto rawCongestionController = mockCongestionController.get();
EXPECT_CALL(*rawCongestionController, getWritableBytes())
.WillRepeatedly(Return(1500));
auto stream1 = conn->streamManager->createNextBidirectionalStream().value();
auto buf = IOBuf::copyBuffer("0123456789");
writeDataToQuicStream(*stream1, buf->clone(), false);
EXPECT_NE(WriteDataReason::NO_WRITE, shouldWriteData(*conn));
// Artificially limit the connection flow control.
conn->streamManager->addLoss(stream1->id);
conn->flowControlState.peerAdvertisedMaxOffset = 0;
EXPECT_NE(WriteDataReason::NO_WRITE, shouldWriteData(*conn));
}
TEST_F(QuicTransportFunctionsTest, HasAckDataToWriteCipherAndAckStateMatch) { TEST_F(QuicTransportFunctionsTest, HasAckDataToWriteCipherAndAckStateMatch) {
auto conn = createConn(); auto conn = createConn();
EXPECT_FALSE(hasAckDataToWrite(*conn)); EXPECT_FALSE(hasAckDataToWrite(*conn));

12
quic/api/test/QuicTransportTest.cpp
View File

@ -270,17 +270,7 @@ TEST_F(QuicTransportTest, NotAppLimitedWithLoss) {
auto stream = transport_->createBidirectionalStream().value(); auto stream = transport_->createBidirectionalStream().value();
auto lossStream = transport_->createBidirectionalStream().value(); auto lossStream = transport_->createBidirectionalStream().value();
auto lossStreamState = conn.streamManager->findStream(lossStream); conn.streamManager->addLoss(lossStream);
ASSERT_TRUE(lossStreamState);
auto largeBuf = folly::IOBuf::createChain(conn.udpSendPacketLen * 20, 4096);
auto curBuf = largeBuf.get();
do {
curBuf->append(curBuf->capacity());
curBuf = curBuf->next();
} while (curBuf != largeBuf.get());
lossStreamState->lossBuffer.emplace_back(std::move(largeBuf), 31, false);
conn.streamManager->updateWritableStreams(*lossStreamState);
conn.streamManager->updateLossStreams(*lossStreamState);
transport_->writeChain( transport_->writeChain(
stream, IOBuf::copyBuffer("An elephant sitting still"), false, nullptr); stream, IOBuf::copyBuffer("An elephant sitting still"), false, nullptr);
EXPECT_CALL(*rawCongestionController, setAppLimited()).Times(0); EXPECT_CALL(*rawCongestionController, setAppLimited()).Times(0);