Skip to content

Commit

Permalink
Don't log/count event when link timestamp is not udpated.
Browse files Browse the repository at this point in the history
Summary:
At router reboot, link status is added into records by `syncInterfaces`. Link status added has timestamp = 0 because link doesn't come up (more exactly change status yet). It is advertised to itself, and counter is pushed to ODS with very large propagation time. To eliminate this special case, don't push to  ODS if timestamp = 0.

https://fburl.com/ods/6ccgaf4f --> filter very large propagation for down link event -> looks good.

Reviewed By: xiangxu1121

Differential Revision: D55955501

fbshipit-source-id: c34177a38c73d592f87f27fcf3ecf01f9f56fe6f
  • Loading branch information
ngocdmsj authored and facebook-github-bot committed Apr 12, 2024
1 parent 7203602 commit 2d50bac
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
6 changes: 5 additions & 1 deletion openr/decision/LinkState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,11 @@ LinkState::mayHaveLinkEventPropagationTime(
if (adjDb.linkStatusRecords().has_value()) { // link status not recorded
auto linkStatus =
adjDb.linkStatusRecords()->linkStatusMap()->find(linkName);
if (linkStatus != adjDb.linkStatusRecords()->linkStatusMap()->end()) {
// Timestamp is updated when link status changes (over Netlink). But
// at router boot-up, link doesn't change status, and so timestamp is
// not set and equals to 0 (default value). We ignore this situation.
if (linkStatus != adjDb.linkStatusRecords()->linkStatusMap()->end() &&
*linkStatus->second.unixTs()) {
int64_t propagationTime =
getUnixTimeStampMs() - *linkStatus->second.unixTs();
if (propagationTime >= 0) { // ignore negative delta (NTP issue)
Expand Down
10 changes: 10 additions & 0 deletions openr/decision/tests/DecisionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6562,6 +6562,16 @@ TEST_F(DecisionTestFixture, LinkEventPropagationTime) {
counters = fb303::fbData->getCounters();
EXPECT_GE(
counters["decision.linkstate.down.propagation_time_ms.avg.60"], 100);

// Down link event with timestamp is not updated, then it's skipped
fb303::fbData->resetAllData();
adjDb2 = createAdjDb("2", {}, 2);
rec1.linkStatusMap()["2/1"].status() = thrift::LinkStatusEnum::DOWN;
rec1.linkStatusMap()["2/1"].unixTs() = 0;
adjDb2.linkStatusRecords() = rec1;
linkState.updateAdjacencyDatabase(adjDb2, kTestingAreaName);
counters = fb303::fbData->getCounters();
EXPECT_EQ(counters["decision.linkstate.down.propagation_time_ms.avg.60"], 0);
}

TEST(DecisionPendingUpdates, perfEvents) {
Expand Down

0 comments on commit 2d50bac

Please sign in to comment.