[00:32:53] dpifke: I've lost track of a number of patches but for now feel free liberally assign anything to me for review to get ahead of my backlog, at least for a couple of weeks while I get through stuff. [00:36:12] Hm.. this says Unauthorized now; https://performance.wikimedia.beta.wmflabs.org/arclamp/svgs/daily/2021-03-16.excimer-wall.api.reversed.svgz [01:49:55] Beta Swift has issues. Will prioritize sorting it tomorrow. [11:10:12] dpifke: fyi I found the cause for beta swift issues and documented it on T276179, but not sure what's the best way to fix it [11:10:13] T276179: Beta SWIFT seems to be broken - https://phabricator.wikimedia.org/T276179 [15:44:27] Majavah: Huge thanks for figuring out what was up with Swift in beta! [15:59:43] Krinkle: fyi https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/673173 [16:12:06] ori: very nice! [16:18:02] dpifke: happy to help, first time debugging swift and definitelty didn't expect to have an unexpected kernel upgrade to be the root cause :P [21:56:47] https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/673366 should be another 0.8% of wall-clock time [22:02:49] the php.incrementStatsKey() calls in the Wikibase Lua module are another 0.15% (https://github.com/wikimedia/Wikibase/blob/2f08ce07a64c78cc4857f54efbde1bdff76b169c/client/includes/DataAccess/Scribunto/mw.wikibase.lua#L120) [22:03:28] that's quite a bit of difference [22:03:48] what do you mean? [22:04:29] I mean, perf gain for relatively simple changes [22:05:08] I'm not booking any wins until we see results in prod, but yeah, looks like it [22:06:49] we can't avoid calling into PHP for incrementStatsKey(), but maybe we can just get rid of those calls. [22:07:04] they were probably added with the presumption that they'd be cheap [22:21:53] https://phabricator.wikimedia.org/T277817 for incrementStatsKey() [23:37:47] Krinkle: quick question if you have a minute. If I have an IDatabase and I wanna check if it's a master or replica connection - is there a way to do that? [23:40:28] Pchelolo: not really, it supposed to be known ahead of time [23:40:53] plus it could conceptually be replica but actually master on a wiki with a single DB server etc. [23:41:23] hm.. with this actor store patch, I'll keep working on that, and we have the IDatabase passed in, so I wanted to avoid falling back to amster if it's already master [23:43:21] ok. thank you. I guess just falling back always. plus a local non-evicting cache for the actors we just wrote [23:43:37] Pchelolo: right, but we generally structure the logic differently, might make sense to change here to follow that for consistency. [23:44:06] e.g. if it's meant to just run a query, then the caller should be responsible for trying a different way. If it's mean to handle both, then it should probably be given both, or be given an $lb object [23:46:03] the thing here is that actor_id are mostly used as a part of a transaction on some other table, that's why we changed it to take IDatabase instead of READ_* constant.. [23:46:32] I commented on the task - if that's the case then you don't need a fallback to master [23:46:47] since the $db handle would be the master handle in that case right? [23:47:11] and if not, then you know it wasn't written to recently. [23:48:04] yeah.. I guess on write we will always use acquireActorId, and on read passing the DB is not really nesessary. [23:48:31] ok. I'll poke at it more and will add you to the followup review [23:48:33] I assume this is a private method, right? callers outside the ActorStore would not know what db handle to pass or why. [23:48:49] the store class decides what db handle to pass, presumably based on whether there were writes already in the same request [23:49:09] or if you follow daniel's idea, then you can always query from replica only, and use a process cache for recent additions instead. [23:54:18] ok. thank you.