Attention is currently required from: neels.
iedemam has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/28165 )

Change subject: stats: new trackers for lchan life duration (v2)
......................................................................


Patch Set 4:

(1 comment)

Patchset:

PS4:
> I don't see any stats reporting in this patch anymore. This is only about VTY 
> output?
> The commit log says "stats".

The function bts_store_lchan_durations() stores duration values in the stats 
system after summing across all lchans. I'm already using these values in 
production and comparing to our previous approach. They are tracking well.

> IMHO it would be better to fix the performance problem in osmo_time_cc.
> The way this patch avoids performance issues is to reduce the timer callback 
> to 1/s,

No, the biggest improvement is only using one timer per BSC instead of one 
timer per lchan.

> Of course we can reconfigure osmo_time_cc to trigger timers only once per 
> second.
> It may be necessary to separate the granularity from the update timer period,
> then we can get milliseconds precision while updating that value less often.
>
> Another fix is to have only one single timer callback that updates all active
> osmo_time_cc. Like that we can have active lchan tracking in millisecond 
> precision
> for *any* number of lchans with only a single timer in select() for all of 
> them.
> This would reduce the select() load by a factor that is the number of active 
> lchans,
> in production environments this is more than factor 10 (as this patch does).

I think we're just turning things inside out at this point then. Why add any 
timer logic at all to each lchan when they will all be externally controlled 
and triggered? The only thing each lchan needs to know about itself is a couple 
of timestamps.

> osmo_time_cc made problems because it is fairly new code that tries to solve a
> pretty complex problem in a generic way, and I built some problems into it.
> IMHO it is worth it to make that work, so that iedemam's patch will perform 
> well.

If you were free to implement all of the above suggestions, I might vote this 
way as well. However, I don't think you have much free time on your hands given 
the delay in each response. This is not a criticism, just being practical in 
solving the problem at hand.



--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28165
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ie3771233ecbd4bc24a24fb22c1064a18e7b8b2b0
Gerrit-Change-Number: 28165
Gerrit-PatchSet: 4
Gerrit-Owner: iedemam <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: laforge <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Comment-Date: Wed, 25 May 2022 12:02:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <[email protected]>
Gerrit-MessageType: comment

Reply via email to