[07:54:25] pfischer: o/ thanks for your review on https://gerrit.wikimedia.org/r/c/wikidata/query/rdf/+/1053938, when you have a moment could you do another pass on it? [08:00:24] sure [08:01:31] thanks! [08:02:38] LGTM! Shall I +2 it? [08:04:12] pfischer: yes please :) [09:06:33] was wondering if we should move T366253 to current work? [09:06:34] T366253: Create a generic stream to populate CirrusSearch weighted_tags - https://phabricator.wikimedia.org/T366253 [09:35:33] dcausse: Sure, we’re running low on things to do for the SUP anyways. [10:11:12] lunch [10:22:35] lunch 2 [12:28:52] not feeling great, will take a break til pairing [13:18:37] o/ [13:48:19] \o [13:53:03] ryankemper I won't make pairing today, are you able to ban/depool elastic110[0-2] and wdqs1016 for T348977? If not I can do it now [13:53:03] T348977: Upgrade EVPN switches Eqiad row E-F to JunOS 22.2 - https://phabricator.wikimedia.org/T348977 [14:04:25] o/ [14:17:34] * ebernhardson is mildly surprised to find opensearch 1.x nodes and elastic 7.x nodes can be in the same cluster for a rolling upgrade [14:17:46] makes sense, but still mildly surprising [14:18:30] nice! [14:21:22] yeah, I was looking at that too...guessing Observability has actually done it w/their ES->OS migration, but we should probably check w/them once we're closer [14:30:42] dcausse looks like we have approval, will get started shortly [14:31:55] gehel: wmde sync? [15:03:44] pfischer: if you're around: https://meet.google.com/eki-rafx-cxi (talkign about search metrics) [15:24:55] dcausse do you know what replication factor we want for the topics? Guessing 3? [15:26:53] inflatador: the default which I think should be 3 copies? but I'd rather ask Andrew [15:27:58] ottomata ^^ is 3 the default replication factor for topics at WMF? [15:28:31] based on `kafkacat -L` it looks that way [15:35:52] I'm gonna go with that, we can always change later [15:43:30] yes [15:44:07] 15:43:47 [@kafka-main1003:/home/otto] $ sudo cat /etc/kafka/server.properties | grep replication.factor [15:44:07] # The default replication factor for automatically created topics. [15:44:07] default.replication.factor=3 [15:49:17] inflatador: yeah I got it [15:52:03] if I wanna look at existing topic config, what'st the best way to do that? I've been using ` kafka configs --entity-type topics` ...looks like most topic config are empty. Some do show retention though: https://phabricator.wikimedia.org/P66749 [15:52:27] ryankemper ACK, thanks [15:54:24] also wondering if we could/should enable topic compaction, re T354794 [15:54:24] T354794: Requesting permission to enable kafka log compaction for page_rerender on kafka-main - https://phabricator.wikimedia.org/T354794 [16:01:09] inflatador: no idea on the kafka Q, but we've still got plenty of ppl in weds meeting if u wanna pop by to ask it https://meet.google.com/eki-rafx-cxi?authuser=1 [16:01:40] inflatador: that is the best way. most topics don't have any special configs [16:01:43] the ones that do have been manually applied [16:02:08] ryankemper nah it's all good. I'm going to go ahead and create the topics with the log compaction enabled, as disk space is tight on the kafka machines ATM. We can always alter it later [16:03:03] inflatador: what are the topics for? if the consumer isn't expecting compaction, things could be weird [16:03:22] if yall know you want it go for it, just checking it isn't done without considering that [16:04:45] ottomata dcausse per https://phabricator.wikimedia.org/T354794#9502455 , compaction was added for the cirrus updater topics, if this is not acceptable for the WDQS updater topics (or if you'd just prefer I don't do it) LMK. Was thinking about saving disk space [16:05:27] inflatador: no compaction for these ones we don't ship message keys [16:05:41] dcausse ACK, will not enable then [16:12:53] dcausse OK, I made the topics w/4 wk retention on kafka-main. LMK if there are any issues [16:13:09] inflatador: awesome, thanks! [16:13:37] np! Working out, back in ~40 [16:23:36] inflatador: just checked and we need these exact same kafka topics in kafka-main@codfw too [17:03:23] back [17:03:23] dcausse oh yeah, duh...on it [17:09:39] dcausse OK, should be good now [17:11:35] inflatador: thanks! I don't yet see codfw.rdf-streaming-updater.mutation-scholarly tho [17:12:16] inflatador: nvm I see it now! [17:12:26] thanks! [17:15:22] heading out for the day...see you in ~24h [17:26:35] dinner [17:33:16] ebernhardson: i think the wording on the related articles thing is resolved with the addition of one word and subtraction of another. It currently says "The clickthrough ratio is potentially lower than reality." It was trying to convey "The clickthrough ratio displayed is lower than reality." Does that clarify it? I think I now see the ambiguity! [17:53:42] dr0ptp4kt: ahh, yea that should work [17:54:32] ty. i'll email myself a reminder on this, which would be good to do at the same time as the cross-link to the android dashboard. superset was being finicky last time i touched anything in the panels, so i want to get it done all at once. [19:22:42] * ebernhardson begrudgingly sets textwidth=95 to match the width of README.md in EventBus. but the lines are too long :P [19:28:57] ebernhardson: patches accepted :) [19:30:56] ottomata: i'm working on a patch, it's easy enough to make it all 75 chars if you don't mind [19:31:06] the patch is unrelated, would be a pre-patch i suppose [19:50:56] i don't mind! [19:52:34] ebernhardson: I don't totally understand your recent comment :) [19:52:43] as is, the stream name is totally hardcoded, ya? [19:53:05] the intention is to be able to configure the stream name per wiki, and use a different stream name for private wikis [19:53:25] the default will still be hardcoded. [19:53:38] it will only be overridden on private wikis (or in cases where you might need to, e.g. during a migration or something) [19:53:38] ottomata: i mean that we will have `mediawiki.page_change.v1` in the code, and `mediawiki.page_change.private.v1` in the config, then when a new version is deployed that changes it to `mediawiki.page_change.v2` that will only be correct for the newly deployed branch [19:53:50] ottomata: but the config isn't per-branch [19:54:26] so i was thinking to only resolve the initial name portion, and append the version basically [19:54:54] if there is an override in config, ya you'll have to change config too [19:54:55] i see. [19:55:11] i think: .v2 would be a totally new stream [19:55:46] so if you are adding it to eventbus, you'll have to emit it as a new stream. perhaps what we need is to name the config variable accordingly [19:55:55] ottomata: hmm, if it would be a completely new implementation that might be ok as is [19:56:04] e.g. instead of EventBusStreamNamesMap['mediawiki_page_change'] [19:56:06] we did [19:56:08] EventBusStreamNamesMap['mediawiki_page_change.v1'] [19:56:32] so in private wiki it'll be overridden to [19:56:47] EventBusStreamNamesMap['mediawiki_page_change_v1'] = mediawiki.page_change.private.v1 [19:56:58] that way if/when you have to add a v2, you can do it alongside of v1 [19:57:04] as a completely new thing [19:57:34] i'm also not entirely sold on having different names in the map and the stream, wouldn't it be simpler if the map was also using `mediawiki.page_change.v1` instead of `mediawiki_page_change_v1` ? [19:59:20] ottomata: the first part makes sense, i can keep the version number all together then. [20:01:09] ebernhardson: i think that could be fine. so the idea being that the key in the map should match the default name of the stream? [20:01:42] ottomata: i suppose other thoughts, does it really have to be implemented per hook, it seems the implementation could potentially go into EventBus::send? There it accepts the stream name, and we can adjust $event['meta']['stream'] to match since it's standardized [20:01:54] OOOOOO [20:01:58] ottomata: yes, the idea would be to put the direct stream name as the key in the map, keeps it simpler [20:02:01] now we getting fancy [20:02:03] :P [20:02:17] > direct stream name as the key in the map [20:02:17] +1 [20:02:43] I'm not sure what I think about having send() mutate the event [20:03:17] I guess i was thinking about EventBus::send because i started to create a StreamNameMapper class, but it has like 3 lines of code which is silly. But then repeating it in a few places also seems a bit silly. But we can stay as is [20:03:19] but we could have a standardize function for this that each hook could call before calling send [20:04:00] btw https://gerrit.wikimedia.org/r/c/mediawiki/extensions/EventBus/+/1053785/2 [20:04:02] still very WIP [20:04:05] i'm playing with some other refactors too [20:04:27] like https://gerrit.wikimedia.org/r/c/mediawiki/extensions/EventBus/+/1054934/1 [20:04:28] i dunno though. [20:05:07] The first i would have to think more about, the second makes sense though. It wasn't really clear when starting to look over this why we needed both [20:10:10] the first is kinda needed for the second. Allows to get rid of the EVENT_SERVICE_DISABLED_NAME EventBus instance that is used when a stream is disabled [20:16:35] ok, that makes sense [21:31:30] * ebernhardson misses the java functional abilities in php a bit...it's nice to pass a Function or Provider instead of an entire class with a single public method