Page MenuHomePhabricator

incrementStatsKey() calls in Wikibase Lua are expensive
Closed, ResolvedPublic

Description

Approximately ~0.15% of MediaWiki wall-clock time is burnt on incrementStatsKey() calls in Wikibase's mw.wikibase.lua and mw.wikibase.entity.lua:

(MediaWiki - daily - 2021-03-18.excimer-wall.all - reversed)

These calls are expensive because they call into PHP from Lua.

Are the Wikibase Lua function usage metrics needed?

Event Timeline

Lydia_Pintscher added subscribers: hoo, Lydia_Pintscher.

@hoo Is this just for stats (and if so which ones?) or also usage tracking?

@hoo Is this just for stats (and if so which ones?) or also usage tracking?

This is just for statistics:
https://grafana.wikimedia.org/d/jO1rYjwmz/wikidata-lua-function-usage?orgId=1
https://grafana.wikimedia.org/d/8HcRs-wZz/terms-related-wikidata-lua-function?orgId=1 (still used?)

I don't think we use these keys outside of the above dashboards.

Easy "fix" would be to have a wrapper function in Lua that only let's every Nth incrementStatsKey call through to PHP (then we would just need to multiply the numbers in the dashboards by N and this should be fine, although depending on the N chosen we loose quite some resolution in our data).

I like this idea, but wouldn't N reset to zero every time we called into Lua code? How would you persist it?

An alternative approach would be random sampling: e.g., log only if math.random(100) == 1.

...and if you increment the stat by a 100 each time, you wouldn't need to change the dashboards.

By the way: this would be a good first patch for a new contributor, and I'd be happy to mentor someone through it.

Change 674472 had a related patch set uploaded (by Ori.livneh; author: Ori.livneh):
[mediawiki/extensions/Wikibase@master] Add a sample rate for Lua function call tracking

https://gerrit.wikimedia.org/r/674472

Ladsgroup added a subscriber: Ladsgroup.

Given that our numbers are large enough. We can simply sample it instead. My only concern is the discontinuity of the graphs atm.

The change I'm proposing (I6b36c1647) should not introduce any significant discontinuity, since the counter delta is proportional to the sampling rate: i.e., with a sampling rate of 1%, 1:100 incrementStatsKey() calls will be processed, but each one will increment the counter by 100.

Ah, I see. Of course, nice trick. I think the change is good to go.

Change 674472 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Add a sample rate for Lua function call tracking

https://gerrit.wikimedia.org/r/674472

Change 676435 had a related patch set uploaded (by Ori.livneh; author: Ori.livneh):

[operations/mediawiki-config@master] Wikibase: sample function call counters at 1:100

https://gerrit.wikimedia.org/r/676435

Change 676435 merged by jenkins-bot:

[operations/mediawiki-config@master] Wikibase: sample function call counters at 1:100

https://gerrit.wikimedia.org/r/676435

Mentioned in SAL (#wikimedia-operations) [2021-04-07T23:10:07Z] <urbanecm@deploy1002> Synchronized wmf-config/Wikibase.php: 321bf91da7e823f026c2c2bdcc57d8cf60a52ba5: Wikibase: sample function call counters at 1:100 (T277817) (duration: 01m 08s)