dorjesinpo
dorjesinpo
> lgtm, what performance stats does this affect? This affects latencies in the (capmon) tests with multiple queues in the same cluster (cluster thread is a bottleneck)
> This looks good to me, can we update with the latest? There is test failure - `test_queue_close.py::test_close_while_reopening[multi_node]` - that seems to be relevant. On the first glance, timing changes...
> Aren't they the same as Storage::numMessages() and Storage::numBytes? In my test they Looking at the `FileBackedStorage::numMessages` they are the same _if_ asking for the queue stats and not appId...
> Regarding `Can we get rid of QueueEngineUtil_AppState::head() and QueueConsumptionMonitor::SubStreamInfo::d_headCb now?` `d_headCb` is `QueueEngineUtil_AppState::head()` which is owned by the QueueEngine. So, there seems to be no need to pass it...
> `QueueConsumptionMonitor::onTransitionToIdle` is edge triggered, so `onTransitionToIdle` is called only once when state is transitioned from `Active` to `Idle`. Ok. This can flap, we often see a lot of subsequent...
> Have some comments, the most important one, are we sure that the loop over downstreams is actually slow? For the ReliableBroadcast, yes. Consider thread switching too. AtomicInt seems to...
Reverting to the old way of counting. A relevant PR is [https://github.com/bloomberg/blazingmq/pull/399](https://github.com/bloomberg/blazingmq/pull/399)