[09:41:54] Errand + lunch [09:53:31] ditto [15:18:23] \o [15:18:26] FYI, we just got tagged on a train blocker: https://phabricator.wikimedia.org/T285951 [15:25:45] hmm, looking but not sure how we could have changed anything relatd [15:27:40] Hi ebernhardson, just want to check in before stopping hadoop for planned maintenance that it's not going to cause a problem for you all to clear the cluster shortly? [15:28:43] razzi: looks ok, the currently running jobs can start again later [15:32:59] ok great, thanks [15:45:13] cbogen_: I took a look at T285951 and the good news (for us) is that it appears that the sections that are red-linked do actually exist, so it's *probably* a problem with the link checking, not out-of-date search indexes. [15:45:14] T285951: Some section links in search results are redlinks - https://phabricator.wikimedia.org/T285951 [15:45:38] thanks for looking at it ebernhardson and Trey314159 ! Is there another team that we should tag in that case? [15:45:54] yea i'm not finding anything interesting relatd to how those work, if i need to i can setup some debug tooling on an mwdebug host but thats always a pain... [15:46:55] I don't know who owns that function. [15:47:38] generally i would assume link rendering is related to parsing, but not really sure [15:48:00] that may be an old an no longer relevant distinction :) [15:48:03] hmm i don't know anyone on parsing to tag and ask [15:57:56] cbogen_: it seems to be already being addressed on the task [15:58:14] with a question on how to test it (dcausse, ebernhardson ?) [15:59:16] unrelated: I've asked access to security tasks to all team members who don't have it (and yes, I include cbogen_ and mpham as team members) [15:59:17] T285947 [15:59:18] T285947: Security Issue Access Request for Search Platform team members - https://phabricator.wikimedia.org/T285947 [15:59:35] dinner, back later [16:18:52] dinner [16:19:13] thanks gehel! [21:00:33] ryankemper: so how's everything going with readaheads? Wondering if we need to put in mitagtions while on vacation [21:01:21] at the easiest level, run elasticsearch-madvise-random against the elasticsearch pid every 30 minutes should keep things from alerting if we can't get a better solution in place. [21:05:31] ebernhardson: agreed, we should put that in place at a minimum; right now i'm taking a look at a codfw host and trying to see if/why its readahead isn't respected. the readahead works for cloudelastic - when we deployed it months ago we saw the IO drop correspondingly - so it might be a simple matter of the wrong device in hiera or something [21:05:53] hoping to fix the actual problem but we should open up a patch for the every 30 minutes right now so we've got that ready if we need it [21:06:05] ebernhardson: how do we normally do that kind of thing, systemd timers? classic cron? or something else [21:06:18] ryankemper: well, we only did once before. and i used tmux sessions :P [21:06:40] while true; sleep 30; ./elasticsearch-madvise-random $(pgrep java); done [21:06:45] 39m [21:06:54] bah, something like that :) We can use systemd timers though probably [21:07:16] that would work fine...we'd have to manually set it up on each of the 36 codfw hosts though right? [21:07:22] i think i was fixing that while everyone was at wikimania, was easier to do it myself than find someone to merge things [21:07:43] i used `clusterssh` to create a bunch of windows all controlled from one input prompt [21:08:23] presumably an old script floating around somewhere, since I don't see anything on wikitech about it? [21:08:37] something like: clusterssh $(for i in $(seq 2001 2070); do echo elastic$i.codfw.wmnet; done) [21:08:55] no, just typing things in :P [21:08:59] oh clusterssh is a normal linux thing [21:09:12] yea, its a probably quite old thing [21:10:29] systemd timers would probably be cleaner, and you can merge things :) But either way [21:11:20] yeah let's go the timer route since it makes it easier for any other SREs to see what's going on [21:11:36] ebernhardson: is `elasticsearch-madvise-random` already placed on each host? do we have it in puppet or something? [21:11:55] we would have to drop the executable in puppet i imagine, with a link to the code on phab. i can find it, sec [21:12:42] The code is https://phabricator.wikimedia.org/P5883 . I should have mentioned git hashes or versions for those two dependencies...but oh well [21:13:16] the tool is a 30kB binary, not great to drop into puppet. hmm [21:18:42] yeah it does seem rare to have bigger files in puppet...we have a `files/puppet` and `files/ssl`, so potentially we could create a `files/cirrus` and drop the file in there [21:19:26] then the path would look like `puppet:///files/cirrus/elasticsearch-madvise-random` [21:20:07] seems reasonable. The tool has come in handy a few times over the years might as well put it somewhere [21:21:07] okay working on that patch [21:22:11] i suppose the $(pgrep java) won't work these days, there are three elastic processes. Will want to check the pid files probably [21:25:57] ebernhardson: where's the logical place to have puppet stick the file on the actual host? `/bin/`? [21:26:07] ryankemper: probably /usr/local/bin/ [21:26:27] ah, yes [21:26:38] in theory i think anything not managed by the package manager is supposed to be in /opt/{package} or /usr/local/ [21:37:09] ebernhardson: https://gerrit.wikimedia.org/r/c/operations/puppet/+/702754 (wip): still need to add the systemd timer part, and need to copy the actual binary in, and (minor) need to dig up the phab ticket for the general load issue for the comment (it's in the scrollback buffer a few days up so I'll grab it after we get the other stuff hashed out) [21:52:06] * ryankemper was rusty on timers, thankfully the arch wiki always comes in clutch [21:52:55] I was thinking of timers as being able to execute arbitrary stuff, but it looks like they trigger another unit (by default, a service of the same name) so basically we'll have a `disable-elasticsearch-readahead.service` and `disable-elasticsearch-readahead.timer` [21:55:27] hmm, i thought they could execute arbitary things as well. hmm [21:56:07] There's a hacky way to do it but it's no bueno: https://wiki.archlinux.org/title/Systemd/Timers#Transient_.timer_units [21:56:41] ryankemper: via systemd::timer::job in puppet you should be able to execute arbitrary shell from `command`? I remember using these before for managing dropping data in hdfs before airflow [21:58:48] ebernhardson: ah, didn't know we had an abstraction for that, sweet. yeah it looks like under the hood it's creating the service and timer units [21:58:56] https://www.irccloud.com/pastebin/8PzbJraJ/ [21:59:29] ahh, gotcha [21:59:44] Will switch to that though...was writing out the two unit files manually, this is much easier :P [22:03:07] * ryankemper is pondering that age-old question: do I name it `disable-elasticsearch-readahead` to make it more readable or `elasticsearch-disable-readahead` so that it starts with `elasticsearch` [22:03:20] think i'll go with the latter [22:05:53] heh most of the other teams' timers i'm looking at it are treating it exactly like cron, giving it something like `'interval' => 'Mon *-*-* 3:15:0'` [22:06:01] We don't want that though, we just want a simple monotonic "every 30 minutes" [22:06:49] In the actual timer file what we want is `OnUnitActiveSec=30min` (will run 30 minutes after the last run, forever), so trying to understand what I need to give `systemd::timer::job` to get that [22:08:24] Ah it sounds like maybe under the hood systemd timers need both the `OnUnitActiveSec` for the "every X minutes" logic but also an `OnBootSec` for when to start relative to the first boot [22:08:37] If so that explains this comment here where it fills it in as needed to get systemd to do the right thing: [22:08:52] https://www.irccloud.com/pastebin/QPYg7vGd/systemd%3A%3Atimer%3A%3Ajob%20snippet [22:09:06] (I'm just stream-of-consciousing by the way, I've almost got it figured out :P) [22:09:25] huh, sounds annoying :P [22:09:59] Yeah...looks like it saved me from a potential foot-gun because I was going to just define `OnUnitActiveSec` when I was doing it manually (I would have caught it after deploying and seeing the timer not fire but still) [22:18:47] Okay I think I understand now...there's `OnUnitSec`, `OnActiveSec`, and so `OnUnitActiveSec` is probably both `OnUnitSec` and `OnActiveSec`, so TL;DR there wasn't actually a footgun [22:19:18] That being said I think I need to set `OnBootSec` to 1 second, otherwise the timer won't fire until 30 minutes after we create it (so 30 mins after the first puppet run) [22:21:15] Nope `OnBootSec` is when the host is booted not when the timer is created, duh [22:22:16] Ah but per `systemd.timer(5)`, `If a timer configured with OnBootSec= or OnStartupSec= is already in the past when the timer unit is activated, it will immediately elapse and the configured unit is started. This is not the case for timers defined in the other directives.` ...so yeah I'll want `OnBootSec` set to 1s [22:39:45] ebernhardson: Okay I've almost got it ready...where can I grab the binary from? Is it sitting in your home dir on one of the hosts or something? [22:41:23] ryankemper: yea, 2054.codfw.wmnet:~/elasticsearch-madvice-random [22:41:33] i guess i've just been pulling that forward from other home dirs for a few yeras [22:42:19] given we're not entirely sure what versions of deps it's using, it beats having to recompile the binary :P [22:43:52] i dunno, scroll back in the commit history and line it up with the date on the paste :P [22:44:41] oh yeah we totally could figure it out if we wanted to [22:44:54] ...but do we want to? I'll pass for now xD [22:45:20] lol, yea can pass for now. [22:45:31] ebernhardson: I've been assuming we want to run it as `root`, does that sound right [22:48:08] ebernhardson: oh, and we need to make the modification for the multiple PIDs you mentioned, I might need some help with that [22:48:42] otherwise I think https://gerrit.wikimedia.org/r/c/operations/puppet/+/702754 is almost ready to go...just need to address some linter complaints [22:49:47] ryankemper: pretty sure we have to run as root to use ptrace, altohugh i didn't test otherwise [22:49:55] usually it's gated because it lets you control other processes [22:50:05] for pids, sec [22:51:05] ryankemper: you can get the pids from /var/run/elasticsearch/*.pid, should be one for each cluster. [22:52:17] ebernhardson: so I think I'll want a simple wrapper shell script which the timer/service will actually call, and then that script will hand off to the binary for each file in `/var/run/elasticsearch/*.pid` basically? [22:53:22] ryankemper: hmm, i suppose the easy answer is a bash script yea. If this was long term we would probably create a systemd::timer::job per-cluster [22:53:44] i'm not completely sure where that would go though, i guess elasticsearch::instance, but probably not necessary for a temp mitigation [22:54:38] Yeah I'm tempted to just have a file resources in `cirrus.pp`, same place where the job/file resources are in the proposed patch [22:55:03] that reminds me I need to sanity check that `profile::elasticsearch::cirrus` is inherited by each host [22:55:14] Otherwise I'm sticking stuff in the wrong place in puppetland [22:56:26] seems acceptable :) [22:56:28] LGTM: [22:56:30] https://www.irccloud.com/pastebin/zcpkw2mQ/ [23:00:57] sound sgood [23:08:37] Okay wrapper should be doing its job properly: [23:08:39] https://www.irccloud.com/pastebin/M0fIwrLB/ [23:11:02] great [23:11:47] As a side note, not sure if shellcheck is parsing wrong or if the double quotes don't work how I think they do with `$()`: [23:11:51] https://www.irccloud.com/pastebin/iODvWMNF/ [23:12:46] ryankemper: its saying that if `f="a b"` then `cat $f` is `cat a b` instead of `cat "a b"` [23:12:53] Usually as long as you have `""` around the whole thing nothing bad can happen, but maybe because `$()`is executing the command it doesn't "preserve" the quotes inside it? [23:13:02] so you want "$(cat "$f")" [23:13:16] ebernhardson: and it doesn't need a `\` in front of the inner quotes or anything? [23:13:23] ryankemper: nope, thats the magic of $() [23:13:37] awesome, I learned something new today :) [23:14:14] imo $() is a strictly better ``, but maybe i've just used it so long it does what i expect [23:18:46] no it definitely is [23:19:41] I wasn't aware of that behavior though, so I guess once the parser sees a `$(` it interprets everything literally until it hits a closing `)` [23:20:52] tbh i'm not 100% sure, but for the most part ya. you can also nest them [23:43:51] ebernhardson: patch should be ready. I just asked in #sre if anyone is around to review, but given the time of day we should probably ship it now (or at least very soon) and verify we see IO go down [23:44:02] https://gerrit.wikimedia.org/r/c/operations/puppet/+/702754 / https://puppet-compiler.wmflabs.org/compiler1002/844/ [23:44:18] looking [23:46:17] One thing that occurred to me is the `files` directory might be the wrong place...might be that these new files should be under module [23:46:31] For example here's a random shell file for an unrelated service: [23:46:33] https://www.irccloud.com/pastebin/Qfk58C0U/ [23:47:34] * ryankemper finds puppet directory structure very confusing [23:48:25] Yeah I think maybe I should move these to `modules/elasticsearch/files` [23:49:14] i'm not really sure, but in modules/elasticsearch/files is fairly typical [23:49:21] the only thing is typically they are text instead of binaries [23:59:31] ebernhardson: so it's been suggested we turn it into a simple deb package so we don't have to put a binary in puppet [23:59:50] sounds like if we have a makefile for it it will [hopefully] be pretty easy to use `dh_make` to make the actual .deb