chenBright commented on code in PR #3611:
URL: https://github.com/apache/brpc/pull/3611#discussion_r4306793567


##########
test/bvar_histogram_unittest.cpp:
##########
@@ -734,11 +735,38 @@ TEST_F(HistogramTest, window_forgets_old_samples) {
     bvar::Histogram h(bvar::Histogram::BucketSchema({10, 20, 30}));
     bvar::Window<bvar::Histogram> w(&h, 1);
     h << 5;
-    sleep(2);
-    // The value recorded 2 seconds ago has left the 1-second window.
-    ASSERT_EQ(0, w.get_value().num);
+
+    // First wait for the sampler to observe the write. An empty window before
+    // the first sampling round must not count as forgetting the sample. Allow
+    // up to 30 seconds at 100 ms per retry on busy test hosts.
+    bvar::Histogram::Value wv;
+    for (int i = 0; i < 300; ++i) {
+        wv = w.get_value();
+        if (wv.num == 1) {

Review Comment:
   The first phase could indeed miss the transient `num == 1` value.
   
   I've changed it to check the sampler's latest cumulative snapshot under its 
mutex in 15692d5. This confirms the write has been sampled even if the window 
has already expired. Both polling phases retain their 30-second timeout.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to