Skip to content

[Nexthop] Fix verifyWRR test flakiness by keeping traffic running - #1587

Open
vvasavada-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:fix-verifywrr-test-flakiness-by-keeping
Open

[Nexthop] Fix verifyWRR test flakiness by keeping traffic running#1587
vvasavada-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:fix-verifywrr-test-flakiness-by-keeping

Conversation

@vvasavada-nexthop

Copy link
Copy Markdown
Contributor

Pre-submission checklist

  • I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • pre-commit run

Summary

Issue

In verifyWRR, the way verification works is:

  • startSendFn -> start traffic
  • sleep -> wait for it to be steady / line rate
  • get and save port stats before
  • sleep -> actual measurement window
  • get and save port stats after
  • verify the rate
395 startSendFn(); <<<<<< 
396 // In tests like QoS scheduler, smaller number of low priority packets might
397 // go through initially but dopped soon in subsequent looping. So, wait
398 // some time for traffic stablizing before collecting portStatsBefore
399 sleep(timeIntervalInSec);
400 for (const auto& port : ports) {
401 portStatsBefore.push_back(getLatestPortStats(port)); <<<<<<<<
402    }
403 sleep(timeIntervalInSec); <<<<<<<
404 if (!keepTrafficRunning) {
405 stopSendFn();
406    }
407 auto index = 0;
408 for (const auto& port : ports) {
409 portStats.insert(
410          {port,
411           {portStatsBefore[index++],
412 keepTrafficRunning ? getLatestPortStats(port)
413                               : getLastIncrementedPortStats(port)}}); <<<<<<<<

For verifyWRR test case, we use keepTrafficRunning=false. As a result, we invoke stopSendFn() which unprograms the routes. Looking at getLastIncrementedPortStats, it does:

  • wait till port stats start incrementing
  • wait till port stats stop incrementing

Basically, we wait until some traffic is there and then traffic stops.

362  HwPortStats AgentHwTest::getLastIncrementedPortStats(const PortID& port) {
363    HwPortStats lastPortStats = getLatestPortStats(port);
364    // wait till port stats starts incrementing
365    WITH_RETRIES({
366      auto currentPortStats = getLatestPortStats(port);
367      EXPECT_EVENTUALLY_TRUE(
368          *currentPortStats.outBytes_() > *lastPortStats.outBytes_());
369      lastPortStats = currentPortStats;
370    });
371    // wait till port stats stops incrementing
372    WITH_RETRIES({
373      auto currentPortStats = getLatestPortStats(port);
374      if ((*currentPortStats.timestamp_() != *lastPortStats.timestamp_()) &&
375          (*currentPortStats.outBytes_() == *lastPortStats.outBytes_())) {
376        return lastPortStats;
377      }
378      lastPortStats = currentPortStats;
379      EXPECT_EVENTUALLY_TRUE(false);
380    });
381    return lastPortStats;
382  }

This is the problem because if routes get unprogrammed before the first wait, we will never see port stats incrementing.

Fix

The fix is to let traffic flow continue i.e. set keepTrafficRunning=true like other SP and WRR tests.

Test Plan

Ran warm_boot.AgentNetworkAIQosSchedulerTest.VerifyWRR multiple times and it passed.

[ PASSED ] cold_boot.AgentNetworkAIQosSchedulerTest.VerifyWRR (69653 ms)
[ PASSED ] warm_boot.AgentNetworkAIQosSchedulerTest.VerifyWRR (57613 ms)
Summary:
   PASSED : 2

Signed-off-by: Vishrant Vasavada <vvasavada@nexthop.ai>
@vvasavada-nexthop
vvasavada-nexthop requested a review from a team as a code owner September 2, 2026 17:00
@meta-cla meta-cla Bot added the CLA Signed label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant