19:00:02 <laanwj> #startmeeting 
19:00:03 <core-meetingbot> Meeting started Thu Dec 16 19:00:02 2021 UTC.  The chair is laanwj. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:03 <core-meetingbot> Available commands: action commands idea info link nick
19:00:07 <achow101> hi
19:00:14 <laanwj> #bitcoin -core-dev Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball
19:00:15 <hebasto> hi
19:00:16 <laanwj> morcos nehan NicolasDorier paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:00:28 <kvaciral[m]> hi
19:00:30 <jb55> hi
19:00:31 <jamesob> hi
19:00:35 <jamesob> wow, thursday already
19:00:37 <laanwj> welcome to what i guess is the last general IRC meeting of 2021
19:00:40 <michaelfolkson> hi
19:00:44 <luke-jr> hi
19:00:46 <ariard> hi
19:00:50 <sipa> hi
19:01:02 <fjahr> hi
19:01:02 <b10c> hi
19:01:15 <lightlike> hi
19:01:47 <laanwj> there haven't been any pre-proposed meeting topics (use #proposedmeetingtopic during any time of the week)
19:01:55 <laanwj> any last minute one?
19:02:06 <achow101> suggested topic: wallet maintainer
19:02:33 <jonatack> hi
19:02:43 <luke-jr> achow101: are you volunteering? :P
19:02:49 <achow101> yes
19:02:55 <laanwj> jamesob: yeah time flies when you're having-- well, time flies anyway
19:02:59 <cfields_> hi
19:03:03 <jamesob> laanwj: haha
19:03:04 <jb55> achow101 it is then, that was easy
19:03:05 <luke-jr> laanwj: lol
19:03:41 <michaelfolkson> That was too easy. NACK to achow101
19:03:42 <laanwj> #topic Wallet maintainer
19:03:42 <core-meetingbot> topic: Wallet maintainer
19:03:43 <MarcoFalke> jamesob is having a rebase party (fun)
19:03:44 <michaelfolkson> jokez
19:03:47 <luke-jr> x.x
19:04:02 <jamesob> MarcoFalke: how did you know?? ;)
19:04:16 <luke-jr> I need to have a rebase party too soon :x
19:04:32 <MarcoFalke> jamesob: I can hear your keyboard
19:04:39 <jamesob> lol
19:04:54 <laanwj> hahaha
19:05:01 <jamesob> nope, that's just PTSD from _actually_ hearing my keyboard two years ago
19:05:13 <luke-jr> ._.
19:05:15 <jb55> rebase party sounds fun but I might have too many conflicts
19:05:21 <cfields_> lol
19:06:05 <MarcoFalke> achow101: Looks like there are no objections. Just promise to merge no bugs, ok?
19:06:06 <laanwj> ok, on topic, ack to achow101 wallet maintainer
19:06:15 <cfields_> ack
19:06:16 <jb55> ack
19:06:18 <michaelfolkson> ACK
19:06:20 <ariard> ack
19:06:20 <fjahr> ack
19:06:21 <luke-jr> should probably post to ML first, but SGTM
19:06:30 <jamesob> ack achow
19:06:41 <laanwj> no, this is a project decision, not a ML decision
19:06:41 <luke-jr> (or not since it's just a Core internal thing idk)
19:06:43 <b10c> ach ackow101
19:06:47 <laanwj> luke-jr: right
19:06:49 <sipa> ack
19:06:51 <jonatack> ACK modulo no bugs :p
19:06:52 <luke-jr> laanwj: right, but some people might not be here
19:06:53 <jamesob> b10c: god bless you
19:06:59 <achow101> MarcoFalke: those aren't bugs, they're features :p
19:07:06 <MarcoFalke> jup bitcoin-core-dev is only for releases
19:07:09 <laanwj> luke-jr: we'll not immediately assign it so other people have time to comment
19:07:09 <kvaciral[m]> ack
19:07:53 <MarcoFalke> There will also be a pull request, which people can comment on
19:07:56 <laanwj> it's not an actual decision in the meeting just an oppertunity to bring up objections
19:07:59 <laanwj> right
19:08:01 <luke-jr> MarcoFalke: good point
19:08:26 <MarcoFalke> I expect that pull to be open for a week at least, maybe longer because holidays?
19:08:50 <laanwj> yeah
19:09:34 <achow101> i'll open it after the meeting then
19:10:17 <sipa> sgtm
19:10:25 <laanwj> oh github has something new, if you click projects you get "projects (beta)" which doesn't actually have the projects... was afraid for a moment they were all gone
19:10:57 <laanwj> #topic High priority for review
19:10:57 <core-meetingbot> topic: High priority for review
19:11:07 <MarcoFalke> can I haz ACK/NACK on #23411 ?
19:11:08 <gribble> https://github.com/bitcoin/bitcoin/issues/23411 | refactor: Avoid integer overflow in ApplyStats when activating snapshot by MarcoFalke · Pull Request #23411 · bitcoin/bitcoin · GitHub
19:11:15 <jamesob> will look
19:11:21 <achow101> #22558 for me
19:11:22 <gribble> https://github.com/bitcoin/bitcoin/issues/22558 | psbt: Taproot fields for PSBT by achow101 · Pull Request #22558 · bitcoin/bitcoin · GitHub
19:11:34 <laanwj> https://github.com/bitcoin/bitcoin/projects/8   7 blockers, 1 chasing concept ACK
19:12:43 <jonatack> review beg for #22932, up since 3 months, has (drum roll...) one concept ack (thanks promag!)
19:12:46 <gribble> https://github.com/bitcoin/bitcoin/issues/22932 | Guard CBlockIndex::nStatus by cs_main, require GetBlockPos/GetUndoPos to hold cs_main by jonatack · Pull Request #22932 · bitcoin/bitcoin · GitHub
19:12:50 <laanwj> MarcoFalke: achow101: added
19:13:08 <jamesob> jonatack: will look
19:13:27 <jonatack> jamesob: thank you
19:13:30 <laanwj> jonatack: some people seem to be confused why it's necessary, was my conclusion from the comments
19:13:51 <fjahr> I would like to re-add #21726 but I am still working on a rebase, so I can ask again but it should be ready later tonight
19:13:57 <jonatack> laanwj: good point. I rewrote the OP to state the issue, the fixes, and how to review/test it
19:13:58 <gribble> https://github.com/bitcoin/bitcoin/issues/21726 | Improve Indices on pruned nodes via prune blockers by fjahr · Pull Request #21726 · bitcoin/bitcoin · GitHub
19:13:59 <MarcoFalke> I've removed #21702 from hi-prio
19:14:02 <gribble> https://github.com/bitcoin/bitcoin/issues/21702 | Implement BIP-119 Validation (CheckTemplateVerify) by JeremyRubin · Pull Request #21702 · bitcoin/bitcoin · GitHub
19:14:09 <laanwj> jonatack: thanks!
19:14:46 <michaelfolkson> #22674 will be reverted? If it is that will need to go back into high prio
19:14:48 <gribble> https://github.com/bitcoin/bitcoin/issues/22674 | validation: mempool validation and submission for packages of 1 child + parents by glozow · Pull Request #22674 · bitcoin/bitcoin · GitHub
19:15:01 <laanwj> fjahr: added
19:15:08 <jamesob> MarcoFalke: just on the basis that you think e.g. ML discussion, "consensus gathering" is needed first?
19:15:10 <fjahr> laanwj: thanks!
19:15:19 <MarcoFalke> also removed #21160
19:15:23 <gribble> https://github.com/bitcoin/bitcoin/issues/21160 | net/net processing: Move tx inventory into net_processing by jnewbery · Pull Request #21160 · bitcoin/bitcoin · GitHub
19:15:37 <ariard> michaelfolkson: i think that's good enough if past reviwers re-ack, will review back this evening
19:15:45 <MarcoFalke> jamesob: No, see my comment on the pull
19:15:47 <michaelfolkson> ariard: Ok
19:16:17 <MarcoFalke> hi-prio for review doesn't make a lot of sense when the pull can't be reviewed due to needing rebase
19:16:45 <laanwj> MarcoFalke: i guess jnewbery isn't going to get around to it for the forseeable future? should we mark it up for grabs?
19:17:11 <sipa> Let's perhaps leave that question up to him.
19:17:16 <jonatack> (correction, the pull i mentioned has two concept acks by promag and MarcoFalke)
19:17:27 <laanwj> sipa: yes, good point
19:18:30 <laanwj> any other PRs? any other topics?
19:18:51 <laanwj> michaelfolkson: the idea is to revert one of the commits, not the entire PR
19:18:58 <luke-jr> achow101: please don't drop BDB support any time soon tho :P
19:19:10 <laanwj> michaelfolkson: see #23793
19:19:10 <jamesob> death to BDB
19:19:11 <gribble> https://github.com/bitcoin/bitcoin/issues/23793 | validation: Revert "de-duplicate package transactions already in mempool" by laanwj · Pull Request #23793 · bitcoin/bitcoin · GitHub
19:19:31 <sipa> i wish BDB a painful, but very slow, death
19:19:45 <laanwj> michaelfolkson:  but according to achow101 it's unreachable anyway
19:20:20 <luke-jr> XD
19:20:25 <laanwj> in any case reverting the entire seems unnecessary, most of the commits did have extensive review, it was only that one that didn't
19:20:52 <ariard> yeah it's not used currently, submitpackage rpc hasn't been yet introduced afaik, though critical path for future package relay support
19:21:14 <achow101> if it were reachable, i would recommend revert because there's a crashing bug, but it's not reachable, so meh
19:21:20 <michaelfolkson> laanwj: Gotcha, thanks. Have you ever thought of giving advance warning that you're intending to merge something? I've always wondered that
19:21:38 <michaelfolkson> Something like "I think this is ready for merge. Intending to merge in say 3 days"
19:21:49 <laanwj> michaelfolkson: i do that sometimes
19:21:55 <michaelfolkson> Ah ok
19:22:16 <luke-jr> these issues are rare enough that reverting when there's a problem is probably a better approach IMO
19:22:29 <laanwj> i made mistake here
19:23:06 <michaelfolkson> It has secondary effect of being like "last orders" or "last chance to review before merging"
19:23:24 <MarcoFalke> laanwj: I think mistakes are impossible to avoid in general
19:23:26 <laanwj> luke-jr: it only makes sense for extensive or somewhat controversial PRs, not for run of the mill ones
19:23:36 <MarcoFalke> I've merged silent merge conflicts twice in the last week or so
19:23:44 <michaelfolkson> But maybe it has downsides, I don't know. I guess the author would generally prefer immediate merging to avoid need for more rebases
19:23:55 <luke-jr> laanwj: right
19:24:07 <MarcoFalke> They are just too hard to catch while merging
19:24:17 <jamesob> merge-freezes and plenty of testing before releases are the only saving graces afaict
19:24:55 <laanwj> MarcoFalke: yes that happens often enough to me too, usually my local build catches them but it's impossible to run for every platform combo
19:25:21 <MarcoFalke> I'll experiment with DrahtBot re-running the CI on weekends
19:25:34 <MarcoFalke> There is less "real" traffic on weekends
19:25:48 <laanwj> that would be useful
19:25:51 <luke-jr> btw, any reason not to set `greedy` in Cirrus CI stuff?
19:26:03 <sipa> greedy?
19:26:07 <luke-jr> and bump up the -j
19:26:12 <MarcoFalke> luke-jr: Sure, go for it
19:26:13 <luke-jr> sipa: lets the VM use more CPU if it's idle
19:26:26 <sipa> oh, cool
19:26:45 <luke-jr> MarcoFalke: k
19:26:47 <MarcoFalke> sipa: If there is idle CPU on the host it will consume it, even if less CPU was requested
19:26:48 <laanwj> thanks to ariard for doing another review round and catching it anyhow
19:27:07 <MarcoFalke> https://medium.com/cirruslabs/introducing-greedy-container-instances-29aad06dc2b4
19:27:25 <sipa> got it... the only reason you wouldn't want greedy is when you're doing benchmarks i assume...
19:27:46 <MarcoFalke> Oh, and we should be cautious about OOM
19:28:06 <luke-jr> hmm
19:28:08 <MarcoFalke> If there is too many CPUs running msan, it will crash the whole VM
19:28:11 <laanwj> benchmarks and too-tight timeouts
19:28:18 <MarcoFalke> memory isn't assigned in greedy
19:28:18 <sipa> right
19:28:22 <luke-jr> MarcoFalke: how about bumping that one job to 8 CPUs / 32 GB RAM?
19:29:05 <MarcoFalke> There is a pool of CPUs that can be used. If one task uses a lot, then other tasks will be scheduled later
19:29:29 <MarcoFalke> Most efficient scheduling (I think) is to use least amount of CPU per task, but many tasks in parallel
19:29:49 <hebasto> ^ agree
19:30:11 <MarcoFalke> Or, if someone has a credit card with unlimited money, we could use that
19:30:21 <MarcoFalke> Maybe 40k-110k per year or so
19:30:35 <luke-jr> could turn off greedy for that one task then I guess
19:30:59 <sipa> Well there is a constant cost (at least cloning repo / configure scripts) which don't run multithreaded.
19:31:14 <sipa> Which puts a limit on how much splitting things up makes sense.
19:32:47 <MarcoFalke> sipa: Jup, that's why the least amount of number of CPUs is the most efficient (highest total CPU usage)
19:33:07 <laanwj> multithreaded configure sounds pretty cursed
19:33:58 <luke-jr> laanwj: eh, in theory it sounds easy (in practice, tho..)
19:34:15 <laanwj> think of the dependency graph :-)
19:34:43 <sipa> Yes, and in theory, there is no difference between theory and practice; in practice however...
19:34:47 <luke-jr> XD
19:34:48 <bitcoin-git> [bitcoin] luke-jr opened pull request #23797: ci: Use Cirrus "greedy" flag to use idle CPU time when available (master...cirrus_greedy) https://github.com/bitcoin/bitcoin/pull/23797
19:35:07 <MarcoFalke> Most of the time only one CPU is used: https://cirrus-ci.com/task/4949915049656320
19:35:18 <MarcoFalke> So I am not sure how much greedy helps us
19:35:19 <laanwj> hehe
19:35:36 <luke-jr> MarcoFalke: I guess we'll see when Cirrus does the PR's CI
19:36:07 <laanwj> right
19:36:10 <sipa> For the functional tests, I've found that (with enough RAM) running more tests in parallel than your number of CPU cores is beneficial.
19:36:10 <luke-jr> hmm, maybe -j60 or something for the tests would be more time-saving?
19:36:30 <sipa> Because the tests spend a lot of time waiting.
19:36:33 <MarcoFalke> another option would be to use self-hosted machines (#21652)
19:36:34 <gribble> https://github.com/bitcoin/bitcoin/issues/21652 | [WIP NOMERGE DRAFT] ci: Switch more tasks to self-hosted by MarcoFalke · Pull Request #21652 · bitcoin/bitcoin · GitHub
19:37:05 <MarcoFalke> luke-jr: No, there is one unit test that takes a long time. It will block a single CPU
19:37:16 <luke-jr> I've found if I forget to specify -j64, builds take a looong time :x
19:37:49 <jonatack> -j 4 or 5 here :D
19:37:50 <MarcoFalke> sipa: Right, CI uses 2x or 4x the -j of the number of CPUs available
19:38:02 <b10c> where / who runs the self hosted CI runners?
19:38:07 <laanwj> jonatack: same
19:38:20 <MarcoFalke> b10c: Drahty right now
19:38:37 <hebasto> b10c: bitcoinbuilds.org
19:38:55 <MarcoFalke> sipa: For example https://cirrus-ci.com/task/4949915049656320 has two CPUs and "MAKEJOBS:-j4" env var
19:39:09 <MarcoFalke> hebasto: That is a separate CI system
19:39:17 <sipa> It has 2 CPUs, or 2 CPU threads?
19:39:30 <hebasto> MarcoFalke: right
19:39:33 <MarcoFalke> 2 CPU threads
19:39:37 <sipa> As in: how many entries would /proc/cpuinfo show?
19:39:41 <sipa> Ok.
19:39:48 <MarcoFalke> aka vCPU
19:40:26 <MarcoFalke> For example https://cirrus-ci.com/task/6075814956498944 is run by Drahty
19:40:27 <sipa> For make I wouldn't go above the actual thread count. But for the functional_test runner you can go significantly above it.
19:40:37 <MarcoFalke> env var: "worker:DrahtBot-small-cpx21-ci-bb-001"
19:40:38 <michaelfolkson> c-lightning has been having CI problems due to tests taking up a lot of memory https://btctranscripts.com/c-lightning/2021-11-29-developer-call/#ci-problems. Core tests generally use less memory and so haven't experienced same problems?
19:40:45 <laanwj> jonatack: i have way too many computers but not one with >8 cores or enough memory for -j64 :)
19:40:51 <jonatack> sipa: added the "-j60 tip for running the functional tests to https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests (near the end) but increased -j for this never worked well for me
19:41:03 <luke-jr> michaelfolkson: eh, Cirrus allows up to 32 GB RAM O.o
19:41:23 <jonatack> laanwj: yes, those purisms don't have many
19:41:23 <MarcoFalke> sipa: I think there is no downside using a higher -j (if you have enough memory)
19:41:37 <jonatack> sipa: (with 32go RAM)
19:41:56 <sipa> jonattack: well, benchmark on your own system which amount works best. I imagine it's a complex function of cpu / diskspeed etc.
19:41:58 <luke-jr> laanwj: mine is only 16 cores, but they're SMT4
19:42:00 <laanwj> jonatack: right, neither do ARM and RV boards for that matter
19:42:10 <b10c> MarcoFalke: thanks, so hetzner cloud (based on cpx21)
19:42:23 <MarcoFalke> b10c: heh, right
19:42:36 <_aj_> jonatack: test_runner.py -j 24 # works pretty well for me with 16GB; usually get one spurious failure though
19:42:46 <laanwj> luke-jr: ye that helps
19:43:00 <jonatack> i do have to use timeout-factor tho, otherwise a few tests time out frequently (rpc_misc mostly, then feature_blockfilterindex_prune and feature_assumevalid)
19:43:34 <sipa> I/O speed matters a lot too.
19:44:02 <laanwj> the way around that is to mount /tmp as tmpfs
19:44:41 <jonatack> maybe my disk encryption doesn't help matters
19:44:44 <jonatack> aj: nice
19:44:59 <laanwj> or at least, whatever temp directory you use for the functional tests
19:44:59 <_aj_> jonatack: (/tmp is on ssd)
19:45:10 <luke-jr> laanwj: I have a zram :p
19:45:33 <luke-jr> not sure I can recommend it though, as it seems Linux is buggy
19:45:48 <luke-jr> (the zram's ext4 fs gets corrupted sometimes)
19:46:38 <MarcoFalke> I also use tmpfs for /tmp
19:46:42 <laanwj> TIL about zram
19:46:56 <sipa> same
19:47:55 <laanwj> i guess that concludes the topic? let's see what #23797 does
19:47:56 <gribble> https://github.com/bitcoin/bitcoin/issues/23797 | ci: Use Cirrus "greedy" flag to use idle CPU time when available by luke-jr · Pull Request #23797 · bitcoin/bitcoin · GitHub
19:48:08 <luke-jr> also handy for putting swap on
19:48:36 <luke-jr> (haven't noticed any swap-zram issues, so I'm guessing the mentioned bugs are probably ext4)
19:49:18 <laanwj> i don't think it makes sense to do a meeting next week (dec 23th) or after (dec 30th), so next one will be jan 6
19:49:54 <sipa> yeah, agree
19:50:22 <michaelfolkson> Cool. Next week's Core PR review club is on multiprocess if anyone wants to use that as an excuse to play around with the multiprocess stuff :)
19:50:24 <michaelfolkson> https://bitcoincore.reviews/10102
19:50:51 <laanwj> michaelfolkson: thanks for the heads up
19:51:16 <laanwj> #endmeeting