16:59:34 #startmeeting network team meeting, 16 september 2019 16:59:34 Meeting started Mon Sep 16 16:59:34 2019 UTC. The chair is ahf. Information about MeetBot at http://wiki.debian.org/MeetBot. 16:59:34 Useful Commands: #action #agreed #help #info #idea #link #topic. 16:59:40 hello network-team o/ 16:59:45 hi! 16:59:47 hi ahf! 16:59:53 our pad is at https://pad.riseup.net/p/tor-netteam-2019.1-keep 17:00:04 hello! 17:00:16 o/ 17:00:35 hi everybody! 17:00:48 if people are interested, then tomorrow we have a gitlab migration discussion meeting at 18 utc 17:01:12 the first discussion point we have is 0.4.2 triage and what we do here 17:01:27 (don't forget the "stuff to do every week") 17:01:34 (assuming we should still do it) 17:01:35 oh, yeah 17:01:37 revert that! 17:01:42 let's start with those things first 17:02:19 lets start with roadmap? :) 17:02:37 ok 17:02:57 is the roadmap up-to-date based on what people are hacking on? 17:03:33 the kanban? 17:03:37 yep, the new kanban! 17:03:39 https://dip.torproject.org/torproject/core/tor/-/boards 17:03:40 yes for me 17:04:13 ok, doesn't sound like anything is too bad. 17:04:16 #30381 and #30901 shouldn't be in review 17:04:28 #26294 has been merge_ready for some time 17:04:58 I've put #29211 in "next" since it's mostly on hold while I work on 042 stuf 17:05:07 needs revision ones goes back into Doing when they are moved to there_ 17:05:10 ? 17:05:14 yes 17:05:16 moving 30381 17:05:18 ahf: if they're being done, yeah. 17:05:27 i have moved them down to doing now 17:05:27 if they're not being done, they go in one of the other columns I think 17:05:30 yeah 17:05:50 moved #30381 to icebox. it's pending work in #30382 from dgoulet. 17:06:04 asn: what do you think we should do with #26294? There are decisions on it that we still need to figure out. Should we try to get it in 0.4.2, or hang on till 0.4.3 is open? 17:06:11 #26294 is a big discussion 17:06:17 im not sure about #26294. 17:06:26 i have expressed my opinion in my latest comment. 17:06:36 asn: #30381 is going to be done after dgoulet finish the other ticket? 17:06:37 i think more people should express their opinion. perhaps teor/dgoulet who do onion services stuff. 17:06:49 gaba: ye. i think dgoulet will take my #30381 to finish up #30382 17:07:00 nickm: which in this case might make sense to delay to 043 17:07:11 it's a bit sad because the branch has been ready for quite a while, but it is what it is... 17:07:44 well, the implementation has been ready, but it seems that the design might need more analysis 17:07:47 yeah 17:08:03 I think it can still be considered for 042 if we decide to do it this way 17:08:14 ok 17:08:32 im still in for doing #26294 17:08:43 i dont think someone has said "no" but there have been concerns 17:08:43 ok 17:08:44 I think 042 might want some kind of fix for #31653.. that's the only circpad bug that affects production, that I am aware of 17:09:02 they are valid concerns because we are talking about a tradeoff 17:09:07 it's about picking a side on this tradeoff 17:09:08 mikeperry: ok, marking as 042-should. 17:09:33 asn: I wonder if we can do something to rehabilitate the old timestamp code. Not with a timestamp per se, but maybe we can figure something out 17:09:40 hm 17:09:41 the two options are "1. Make the machines not use 0ms delay." or "2. Use timer callbacks even in the event of 0ms delays, instead of directly calling the callback" 17:10:05 mikeperry: i was not aware of the ticket, but i would go with (2) 17:10:08 just from what you said here 17:10:08 I think 2 is fine 17:10:18 (unless there are tradeoffs I don't realize) 17:10:41 [a 0ms delay between what and what?] 17:10:59 nickm: not sure how the old timestamp design worked 17:11:06 nickm: i did not even remember this was the case 17:11:17 It was long ago 17:11:27 we sent the current time in the introduce cell, so that cells could expire 17:11:47 and so the replay cache entries could expire too 17:12:01 problem is, the user's view of the current time is a fingerprint 17:12:12 yes, plus lots of issues with skewed clocks etc. 17:12:27 feel like anything that deals with time synchronization over the network is a recipe for trouble 17:12:46 is the possible solution discussion something that makes more sense for #tor-dev afterwards? or does it make most sense now? 17:12:46 nickm: 0ms delay between the packet that was just sent and tthe padding packet we want to send 17:12:59 ahf: i think it does not make most sense now 17:13:05 but it has been a useful discussion nevertheless 17:13:06 asn, ahf: agreed 17:13:12 mikeperry: ok 17:13:18 cool, i don't want to interrupt it, but it sounds like something that can be taken outside of the meeting 17:13:28 great, anything else for the kanban planning? 17:13:33 (im leaving after the meeting fwiw, so we would need to do it at another time) 17:13:42 otherwise we will move to reviewer assignments 17:13:42 asn: ok w me 17:13:49 there is a very long link on the pad where you can see the list 17:14:06 is everybody OK with the size of this list for the week? 17:14:12 I'm ok 17:14:18 i think everyone took one. maybe me and nickm took two. 17:14:29 i gave mikeperry none because of the ESR review, but not sure about that 17:14:35 i see dgoulet have two tickets there. should we move them to someone else? 17:14:38 hm 17:14:47 (IMO yes, especially if they are for 042) 17:14:47 both are hs related though 17:14:57 asn: are there circpad things I should review? I can still fit those in if they are intricate and I should know about them anyway 17:15:10 mikeperry: nope 17:15:16 so wrt dgouelt stuff: 17:15:27 i can takeover something non-hs from someone who is comfortable reviewing hs related code 17:15:28 #25568 has been going on for a while. i dont think we should change that. last update was 5 weeks ago. 17:15:39 i can take #31652 17:15:59 it is not too much then asn? 17:16:06 (If you don't think we should change something, maybe move it to 043 or unspecified?) 17:16:20 oh, it _is_ unspecified. Great 17:16:33 feel free to take #31657 if u want. i think it's super small anyway 17:16:37 #25568 is from neel it seems 17:16:43 i will assign #31652 to me. it's also from neel. 17:16:46 it is mergey_ready already? 17:16:50 i will let #25568 to dgoulet 17:17:09 #31657 appears to be already merged to master, and pending a backport 17:17:10 #31652 is the only one that really needs to be taking from dgoulet now 17:17:18 #31652 is the one we care for? 17:17:21 yeah 17:17:28 i took #31652 17:17:35 ok 17:17:39 ah, great 17:17:42 thanks asn! 17:18:02 and you wanted to get rid of #31657 the backport part? 17:18:04 i can take that 17:18:13 sure but it's also fine 17:18:16 i think it's a trivial thing 17:18:27 i dont see #31657 mentioning backports or anything 17:18:30 it seems to be a refactoring thing 17:18:41 ah, ack 17:19:12 ok, next thing is to look at the 042 status page 17:19:23 https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTorReleases/042Status 17:19:40 this is both one of our weekly tasks, but also the first discussion point 17:19:49 i took a regression ticket that was unassigned there (#31408) 17:20:09 we have a lot of tickets in this list and the deadline is 3 months away and we were trying to figure out how to distribute the load here 17:20:30 we only have a few "must" tickets, but a bunch of "should"s 17:20:30 it would be very nice if people could go over the list and see which tickets they know they would be the best person for the task and take ownership here 17:20:41 yeah, must tickets have priority here 17:21:05 I've tried to make all the "must" tickets "very high" priority, but I could have missed one or two 17:21:08 and we should also figure out if some of the -must tickets needs to be downgraded to -should and if some of the -should tickets must be upgraded to -must 17:21:32 (-should can also get downgraded to -can) 17:21:36 #31091 sounds like something i should take 17:23:02 * ahf takes #10416 too 17:23:12 i have the name and the OS for it 17:23:20 :) 17:24:03 I can try #31107 if nobody else want it 17:25:06 i am not gonna take #31022 yet, but i think i will take it in the future 17:25:13 it seems low priority for me 17:25:19 ack. 17:25:46 Does anybody want to take #31483 from dgoulet? It seems important 17:26:08 I guess I can do that, if you can review asn. (You reviewed the original single-counter-bucket code, right?) 17:26:25 #31483 is not in this list yet? 17:26:28 hm 17:26:37 i dont know much about the counter bucket internals 17:26:42 i reviewed #15516 tho 17:26:44 #31483 is not in the list of unassigned stuff, since it is assigned to dgoulet 17:26:45 i can review yes 17:26:52 ok 17:26:53 ah, right 17:28:38 ok, cool. only non high priority tickets left now 17:29:03 have everybody gone over and checked if any of the tickets that are should right now should be upgraded to -must? 17:30:09 i take the silence as yes 17:30:10 i see only #31611 as "must"? 17:30:28 * ahf looks 17:30:47 i can help with #31611 but it could take lots of effort. does it need to be "must"? 17:30:53 catalyst: we want to upgrade it to must? 17:31:11 it is s31 must, but 042-should 17:31:27 ahf: oh, i see 17:31:53 but we do want to fix it sooner rather than later so we know that our CI won't miss that kind of thing 17:31:53 i don't know if we want to upgrade it to a must? it seems like we want to do it for the sponsor, but maybe not for 042 17:31:59 ahf: yea 17:32:36 I don't think it needs to be must 17:32:38 cool, let's have it mind when we do the next s31 planning session 17:33:03 anybody mind if we move to the next item? 17:33:14 it is the PR requirement discussion on network-team@ 17:33:43 i hope by now that everybody have read it and thought a bit about it. it will impact all of us if we decide to do it 17:34:00 one of the concerns that have been expressed is around the "avoid large PR's" requirement 17:34:26 we could postpone that PR conversation until dgoulet is back? 17:34:44 or not... 17:34:52 i think it is impossible to get to a point hwere we have 100% follow rate of the policy, but i think it is a good reminder to all of us for how we should do most of our PR's 17:35:12 sometimes there will be outliers of course, but i think the big structural change here is if you want to do something big you should get someone to help review the code early on 17:35:22 so it doesnøt get put on someone desk very late in the process 17:35:28 doesn't* 17:35:49 agreed 17:36:00 gaba: i think we can make some progress without david, ideally that i take the parts and try to turn into a policy and then we do the policy process when david arrives 17:36:09 it makes sense 17:36:27 any objections/thoughts/ideas to all of this? :) 17:37:10 if it's impossible to get 100% follow rate, what is the policy gonna dictate? 17:37:19 aka what do we do with the PRs that dont follow the policy? 17:37:21 good practice? 17:37:22 i don't have strong objections to most of the proposals, but could take another look 17:37:36 expectations on what a PR should be 17:37:37 (im worried in particular about volunteer PRs etc.) 17:37:40 and a review 17:37:41 I have thoughts about a strict policy of breaking up PRs for the sake of breaking the up vs just making sure they have prelim review. I am more ok with early review.. breaking them up just for the sake of it increases friction for future changes while things are still being worked out 17:37:43 at some level the reviewers and authors collectively enforce this 17:38:11 asn: I think that we shouldn't enforce all this on volunteers when not necessary 17:38:22 mikeperry: i wouldn't say it has to be a strict policy. just that my experience is that big PRs have a much higher chance of taking a long time to review 17:38:32 mikeperry: we have ahd really nasty times with huge PRs in the past, fwiw 17:38:32 asn: an ideal practice for all of us working together every day to have a good healthy relationship with each other and towards each others contributions? 17:39:07 exceptions is IMO ok if the parties that are involved with it are OK with it. i personally remember having done very large patches for s8 that was probably not very fun for the mergers to get handed over to them after dgoulet and i had done pair wise run for a long time 17:39:08 we help volunteers as needed to improve their work if it's not following our guideilnes, with the understanding that we expect them to improve 17:39:47 ok sounds good 17:39:48 catalyst: +1 17:39:49 nickm: I would be more comfortable finding out what thos ethings are and dealing with those.. if early/prilim review is what would have solved them, that's fine with me 17:40:22 early review is great, but we still have a merger at the end of the chain who probably also want to review the code before it goes in 17:41:30 huge complicated ticket and brach and PR dependencies for large patches just means that developers are spending their time juggling tickets, branches, and PRs for the sake of it. I have also had issues creep into code due to confusion and conflicts from these dependencies also 17:41:55 mikeperry: I think the idea is that the dependencies should not grow that large 17:41:59 things should be merged piece by piece 17:42:21 If it is too big to handle as 10 PRs, it is probably too big to handle as 1 PR 17:42:41 nickm: +1 17:42:48 I am not seeing how that necessarily means 10 PRs is the solution? 17:42:51 the idea with a policy here is to ensure that people have that in mind and hopefully avoid some of those situations, but also to allow other people on the team to say "hey, maybe next time we should have done this in a different way" without it being a bad thing to say 17:43:01 we should be more willing to tell each other "hey that might be too big", and to listen when we receive such feedback 17:43:08 mikeperry: also nobody is saying "do more PRs for the sake of it" 17:43:17 i dont think we are going to avoid all the situations with this, but if we can reduce them that is good too 17:44:13 well our flows and tooling means that things like practracker rules to block PRs over X lines are a likely outcome.. I would be more comfortable with a policy that just has different pproccesses (like prelim review) 17:44:15 mikeperry: nobody's asking for 10 simultaneous PRs. One or two PRs at a time on a big complex thing is probably better 17:44:38 i'm thinking that phrasing it like "if you're afraid you'll lose too much work if someone gives you some review comments that require reworking your code, maybe get prelim review next time, or make PRs smaller" 17:45:21 what is the difference between doing prelim review and getting multiple smaller chunks merged over time? that with prelim review you don't want the merge to happen? 17:45:34 the thing just grows while being reviewed and then at the end it is passed to the mergers? 17:46:04 because as the thing grows, later things become more clear to both reviewer and developer.. and you end up going back to change/refactor the earlier PRs, and mess results 17:46:06 Multiple smaller chunks merged over time also gives the mergers opportunity to notice problems and request changs early 17:46:17 ahf: i would prefer smaller PRs, with the prelim review being the earlier PRs in a series 17:46:31 mikeperry: right 17:46:39 ahf: but sometimes we are in a situation where we already have large complicated PRs in progress 17:46:46 those things are much easier to fix in a prelim revie scenario than a "its already merged go back and fix it and refactor and rebase till you die" flow 17:46:47 yeah 17:47:29 hm 17:47:38 mikeperry: i learned that being willing to rework and refactor after merging is important to reducing tech debt, rather than getting a new architecture perfect on the first try 17:47:49 yes it can seem like more work overall 17:48:21 i have had tickets where they kept growing over a long period where i know for sure they have not been fun for the mergers to deal with. i think if i had paid the price earlier in the process and split things out it would have made the process smoother for everyone else at the expense of some additional work for me 17:48:32 catalyst: it is more work overall.. once things are in more branches we have to do more backports and conflcits result.. esp since big things will tend to span multiple releases 17:48:57 is it more work for the sum of the team members or more work for the person working on this? 17:49:38 I think both.. reviewers will have to review the refactoring tickets too 17:49:48 mikeperry: the idea is not to have a bunch of branches all active at once, 17:49:56 but to review and merge early and often. 17:50:12 i think we shouldn't back port refactoring or architecture changes without really good reason, e.g., security flaw that's not feasible to fix any other way 17:51:04 but this leads to greater branch divergence and conflicts for code that is just nearby.. it is easier of just that large branch skips that release and waits; then only it has to be refactored 17:51:11 also we have less than 10 minutes remaining and i'm not sure we can arrive at consensus in that time 17:51:14 isntead of everything near those line changes 17:51:20 ok, we have sub 10 min left. it sounds like there is at least one hot potato in this. should we continue this on the mL? 17:52:00 +1 17:52:03 +1 17:52:12 mikeperry: are you up for writing your thoughts there? then david can also join in, maybe when he returns if he feels like it 17:52:22 ok, we have some people who needs help with some stuff 17:52:40 the first one is mike. i didnøt check if you were given reviews this week 17:52:46 I can try.. I don't have a lot of spare time this month... 17:52:50 but you would like not to have any reviews right now? 17:53:38 yes, asn mentioned earlier that no reviews for mike this week 17:53:43 great 17:53:44 cool 17:54:18 catalyst have some items related to CI 17:54:41 ahf: i've mostly updated the CI wiki page accordingly. maybe one or two tickets need opening 17:55:08 oki, the jenkins failures are not something that needs help with this week? 17:55:42 ahf: not at the moment. i was leaving it there because teor made some comments 17:55:51 ah, great 17:56:13 swati: hey! 17:56:21 swati: you need help with configure options and getting a trac account? 17:56:40 Hey ahf! 17:56:50 swati: feel free to ask questions to all of us at any time in #tor-dev about these things. i think most people on the network team might be able to help you there 17:57:01 i wonder why trac registration is closed right now though 17:57:07 gaba: you know that? 17:57:07 yes, I need an account to create a pull requests. 17:57:16 pull requests. 17:57:19 no but I'm creating a ticket 17:57:23 mmm 17:57:29 swati: you need an account in trac? 17:57:36 PRs are made in github 17:57:37 yes. 17:57:50 about trac, I'm creating a ticket for your account. 17:57:56 thanks gaba 17:58:04 okay, do we have anything else for the meeting? 17:58:11 Yes, I know. But Taylor mentioned in the email that it is good to also have trac tickets associated with Pull requests. 17:58:13 or have i forgotten something important we have to talk about? 17:58:14 That's why asked. 17:58:18 ahh, perfect. 17:58:22 yes, thanks for asking! 17:58:36 Thanks, Gaba! 17:58:49 I will ask other questions on tor-dev. 17:59:13 cool, i am gonna end the meeting now then 17:59:16 #endmeeting