admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make PacingController circuit breaker configurable.

Browse Source 
We have seen a few instances in a down-stream project where the circuit
breaker is still triggering and causing issues.
This CL makes the threshold configurable and adds more debug logging to
try and get to the bottom of this rarely occuring bug.

Bug: webrtc:11340, b/258509536
Change-Id: I92674d446b926ad66538ff9c8be2a32a3d95b057
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283762
Reviewed-by: Emil Lundmark <lndmrk@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38664}
This commit is contained in:
Erik Språng 2022-11-17 11:46:28 +01:00 committed by WebRTC LUCI CQ
parent 5dd548261f
commit e158b77427
3 changed files with 62 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
modules/pacing/pacing_controller.cc
View File

@ -87,7 +87,8 @@ PacingController::PacingController(Clock* clock,
congested_(false),
queue_time_limit_(kMaxExpectedQueueLength),
account_for_audio_(false),
include_overhead_(false) {
include_overhead_(false),
circuit_breaker_threshold_(1 << 16) {
if (!drain_large_queues_) {
RTC_LOG(LS_WARNING) << "Pacer queues will not be drained,"
"pushback experiment must be enabled.";
@ -141,6 +142,10 @@ void PacingController::SetCongested(bool congested) {
congested_ = congested;
}
void PacingController::SetCircuitBreakerThreshold(int num_iterations) {
circuit_breaker_threshold_ = num_iterations;
}
bool PacingController::IsProbing() const {
return prober_.is_probing();
}
@ -423,12 +428,10 @@ void PacingController::ProcessPackets() {
}
DataSize data_sent = DataSize::Zero();
// Circuit breaker, making sure main loop isn't forever.
static constexpr int kMaxIterations = 1 << 16;
int iteration = 0;
int packets_sent = 0;
int padding_packets_generated = 0;
for (; iteration < kMaxIterations; ++iteration) {
for (; iteration < circuit_breaker_threshold_; ++iteration) {
// Fetch packet, so long as queue is not empty or budget is not
// exhausted.
std::unique_ptr<RtpPacketToSend> rtp_packet =
@ -499,14 +502,30 @@ void PacingController::ProcessPackets() {
}
}
if (iteration >= kMaxIterations) {
if (iteration >= circuit_breaker_threshold_) {
// Circuit break activated. Log warning, adjust send time and return.
// TODO(sprang): Consider completely clearing state.
RTC_LOG(LS_ERROR) << "PacingController exceeded max iterations in "
"send-loop: packets sent = "
<< packets_sent << ", padding packets generated = "
<< padding_packets_generated
<< ", bytes sent = " << data_sent.bytes();
RTC_LOG(LS_ERROR)
<< "PacingController exceeded max iterations in "
"send-loop. Debug info: "
<< " packets sent = " << packets_sent
<< ", padding packets generated = " << padding_packets_generated
<< ", bytes sent = " << data_sent.bytes()
<< ", probing = " << (is_probing ? "true" : "false")
<< ", recommended_probe_size = " << recommended_probe_size.bytes()
<< ", now = " << now.us()
<< ", target_send_time = " << target_send_time.us()
<< ", last_process_time = " << last_process_time_.us()
<< ", last_send_time = " << last_send_time_.us()
<< ", paused = " << (paused_ ? "true" : "false")
<< ", media_debt = " << media_debt_.bytes()
<< ", padding_debt = " << padding_debt_.bytes()
<< ", pacing_rate = " << pacing_rate_.bps()
<< ", adjusted_media_rate = " << adjusted_media_rate_.bps()
<< ", padding_rate = " << padding_rate_.bps()
<< ", queue size (packets) = " << packet_queue_.SizeInPackets()
<< ", queue size (payload bytes) = "
<< packet_queue_.SizeInPayloadBytes();
last_send_time_ = now;
last_process_time_ = now;
return;

7
modules/pacing/pacing_controller.h
View File

@ -156,6 +156,11 @@ class PacingController {
bool IsProbing() const;
// Note: Intended for debugging purposes only, will be removed.
// Sets the number of iterations of the main loop in `ProcessPackets()` that
// is considered erroneous to exceed.
void SetCircuitBreakerThreshold(int num_iterations);
private:
TimeDelta UpdateTimeAndGetElapsed(Timestamp now);
bool ShouldSendKeepalive(Timestamp now) const;
@ -232,6 +237,8 @@ class PacingController {
TimeDelta queue_time_limit_;
bool account_for_audio_;
bool include_overhead_;
int circuit_breaker_threshold_;
};
} // namespace webrtc

26
modules/pacing/pacing_controller_unittest.cc
View File

@ -2115,5 +2115,31 @@ TEST_F(PacingControllerTest, BudgetDoesNotAffectRetransmissionInsTrial) {
pacer.ProcessPackets();
}
TEST_F(PacingControllerTest, AbortsAfterReachingCircuitBreakLimit) {
const DataSize kPacketSize = DataSize::Bytes(1000);
EXPECT_CALL(callback_, SendPadding).Times(0);
PacingController pacer(&clock_, &callback_, trials_);
pacer.SetPacingRates(kTargetRate, /*padding_rate=*/DataRate::Zero());
// Set the circuit breaker to abort after one iteration of the main
// sending loop.
pacer.SetCircuitBreakerThreshold(1);
EXPECT_CALL(callback_, SendPacket).Times(1);
// Send two packets.
pacer.EnqueuePacket(BuildPacket(RtpPacketMediaType::kVideo, kVideoSsrc,
/*sequence_number=*/1,
/*capture_time=*/1, kPacketSize.bytes()));
pacer.EnqueuePacket(BuildPacket(RtpPacketMediaType::kVideo, kVideoSsrc,
/*sequence_number=*/2,
/*capture_time=*/2, kPacketSize.bytes()));
// Advance time to way past where both should be eligible for sending.
clock_.AdvanceTime(TimeDelta::Seconds(1));
pacer.ProcessPackets();
}
} // namespace
} // namespace webrtc