Skip to content

Commit

Permalink
[server] SN read quota versioned stats not initialized after restart
Browse files Browse the repository at this point in the history
The currentVersion and backupVersion of ServerReadQuotaUsageStats are not
set after server restart because handleStoreChanged is invoked for all
stores when the store repo undergoing refresh before we initialize and
register store change listener in ReadQuotaEnforcementHandler (part
of the ListenerService). As a result metrics that depend on current
and backup versions will not show up properly until store is updated.

The fix is to during initialization of ReadQuotaEnforcementHandler we
will invoke handleStoreChanged for all stores after we register store
change listener.

The bug is actually reproducible in existing integration test. However,
it was not caught because the test was broken/misconfigured...
  • Loading branch information
xunyin8 committed Nov 15, 2024
1 parent 697ccf9 commit 9288ee9
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ public void testServerReadQuota() throws Exception {
LOGGER.info("RESTARTING servers");
veniceCluster.stopAndRestartVeniceServer(veniceServerWrapper.getPort());
}
serverMetrics.clear();
for (int i = 0; i < veniceCluster.getVeniceServers().size(); i++) {
serverMetrics.add(veniceCluster.getVeniceServers().get(i).getMetricsRepository());
}
for (int j = 0; j < 5; j++) {
for (int i = 0; i < recordCnt; i++) {
String key = keyPrefix + i;
Expand All @@ -198,7 +202,7 @@ public void testServerReadQuota() throws Exception {
quotaRequestedQPSSum += serverMetric.getMetric(readQuotaRequestedQPSString).value();
assertEquals(serverMetric.getMetric(readQuotaAllowedUnintentionally).value(), 0d);
}
assertTrue(quotaRequestedQPSSum >= 0, "Quota request sum: " + quotaRequestedQPSSum);
assertTrue(quotaRequestedQPSSum > 0, "Quota request sum: " + quotaRequestedQPSSum);
}

@Test(timeOut = TIME_OUT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ public final void init() {
for (Version version: versions) {
customizedViewRepository.subscribeRoutingDataChange(version.kafkaTopicName(), this);
}
// also invoke handle store change to ensure corresponding token bucket and stats are initialized.
handleStoreChanged(store);
}
this.initializedVolatile = true;
}
Expand Down

0 comments on commit 9288ee9

Please sign in to comment.