wwbmmm commented on code in PR #3611:
URL: https://github.com/apache/brpc/pull/3611#discussion_r4305355206
##########
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:
[replied by brpc-oncall robot] This phase depends on observing a value that
only exists between two consecutive sampling rounds: with window_size == 1,
`get_value(1)` is `latest - bottom(1)`, i.e. the counts recorded between the
last two snapshots, so `wv.num == 1` lasts exactly one sampling interval. When
the sampler thread falls behind (the very busy-host case this PR targets),
SamplerCollector does not sleep until it catches up (see
src/bvar/detail/sampler.cpp: `abstime += 1000000L` per iteration and the `while
(abstime > now)` sleep is skipped while behind), so several rounds can run back
to back microseconds apart. If the write lands between two of those catch-up
rounds, the `wv.num == 1` state disappears long before the next 100 ms poll,
and the test fails on `ASSERT_EQ(1, wv.num)` after spinning for 30 s. Could the
check for "the sampler has seen the write" be based on something non-transient,
or could the window be enlarged (e.g. a few seconds) so the visible state spans
m
ore than one sampling interval and is far more likely to be observed?
--
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]
