wasphin commented on code in PR #3547:
URL: https://github.com/apache/brpc/pull/3547#discussion_r4037643838
##########
src/bvar/reducer.h:
##########
@@ -93,17 +121,18 @@ class BabylonVariable: public Variable {
}
T get_value() const {
+ CHECK(!(butil::is_same<InvOp, VoidOp>::value) || nullptr == _sampler)
+ << "You should not call Reducer<" << butil::class_name_str<T>()
+ << ", " << butil::class_name_str<Op>() << ">::get_value() when a"
+ << " Window<> is used because the operator does not have inverse.";
return _counter.value();
}
T reset() {
- if (BAIDU_UNLIKELY((!butil::is_same<VoidOp, InvOp>::value))) {
- CHECK(false) << "You should not call Reducer<" <<
butil::class_name_str<T>()
- << ", " << butil::class_name_str<Op>() <<
">::get_value() when a"
- << " Window<> is used because the operator does not
have inverse.";
- return get_value();
- }
-
+ // Unlike AgentCombiner::reset_all_agents(), reading and clearing the
babylon
+ // counter are two separate steps, so values added in between are
lost. This
+ // only affects explicit reset() by users: sampling of an operator
without
+ // inverse is the only internal user and it runs in a single thread.
Review Comment:
@chenBright Could you clarify the comment about sampling being unaffected?
Although `ReducerSampler::take_sample()` runs in a single thread, writer
threads can still update their thread-local slots between `value()` and
`reset()`, so normal sampling appears able to lose updates too.
If this is an accepted trade-off for the Babylon backend, could we update
the comment to acknowledge that it also applies to normal sampling?
--
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]
