16:59:34 <ahf> #startmeeting network team meeting, 16 september 2019
16:59:34 <MeetBot> 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 <MeetBot> Useful Commands: #action #agreed #help #info #idea #link #topic.
16:59:40 <ahf> hello network-team o/
16:59:45 <gaba> hi!
16:59:47 <nickm> hi ahf!
16:59:53 <ahf> our pad is at https://pad.riseup.net/p/tor-netteam-2019.1-keep
17:00:04 <asn> hello!
17:00:16 <catalyst> o/
17:00:35 <nickm> hi everybody!
17:00:48 <ahf> if people are interested, then tomorrow we have a gitlab migration discussion meeting at 18 utc
17:01:12 <ahf> the first discussion point we have is 0.4.2 triage and what we do here
17:01:27 <nickm> (don't forget the "stuff to do every week")
17:01:34 <nickm> (assuming we should still do it)
17:01:35 <ahf> oh, yeah
17:01:37 <ahf> revert that!
17:01:42 <ahf> let's start with those things first
17:02:19 <ahf> lets start with roadmap? :)
17:02:37 <gaba> ok
17:02:57 <ahf> is the roadmap up-to-date based on what people are hacking on?
17:03:33 <asn> the kanban?
17:03:37 <ahf> yep, the new kanban!
17:03:39 <gaba> https://dip.torproject.org/torproject/core/tor/-/boards
17:03:40 <asn> yes for me
17:04:13 <ahf> ok, doesn't sound like anything is too bad.
17:04:16 <nickm> #30381 and #30901 shouldn't be in review
17:04:28 <gaba> #26294 has been merge_ready for some time
17:04:58 <nickm> I've put #29211 in "next" since it's mostly on hold while I work on 042 stuf
17:05:07 <ahf> needs revision ones goes back into Doing when they are moved to there_
17:05:10 <ahf> ?
17:05:14 <gaba> yes
17:05:16 <gaba> moving 30381
17:05:18 <nickm> ahf: if they're being done, yeah.
17:05:27 <ahf> i have moved them down to doing now
17:05:27 <nickm> if they're not being done, they go in one of the other columns I think
17:05:30 <ahf> yeah
17:05:50 <asn> moved #30381 to icebox. it's pending work in #30382 from dgoulet.
17:06:04 <nickm> 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 <ahf> #26294 is a big discussion
17:06:17 <asn> im not sure about #26294.
17:06:26 <asn> i have expressed my opinion in my latest comment.
17:06:36 <gaba> asn: #30381 is going to be done after dgoulet finish the other ticket?
17:06:37 <asn> i think more people should express their opinion. perhaps teor/dgoulet who do onion services stuff.
17:06:49 <asn> gaba: ye. i think dgoulet will take my #30381 to finish up #30382
17:07:00 <asn> nickm: which in this case might make sense to delay to 043
17:07:11 <asn> it's a bit sad because the branch has been ready for quite a while, but it is what it is...
17:07:44 <nickm> well, the implementation has been ready, but it seems that the design might need more analysis
17:07:47 <nickm> yeah
17:08:03 <nickm> I think it can still be considered for 042 if we decide to do it this way
17:08:14 <asn> ok
17:08:32 <asn> im still in for doing #26294
17:08:43 <asn> i dont think someone has said "no" but there have been concerns
17:08:43 <nickm> ok
17:08:44 <mikeperry> 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 <asn> they are valid concerns because we are talking about a tradeoff
17:09:07 <asn> it's about picking a side on this tradeoff
17:09:08 <nickm> mikeperry: ok, marking as 042-should.
17:09:33 <nickm> 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 <asn> hm
17:09:41 <mikeperry> 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 <asn> mikeperry: i was not aware of the ticket, but i would go with (2)
17:10:08 <asn> just from what you said here
17:10:08 <nickm> I think 2 is fine
17:10:18 <nickm> (unless there are tradeoffs I don't realize)
17:10:41 <nickm> [a 0ms delay between what and what?]
17:10:59 <asn> nickm: not sure how the old timestamp design worked
17:11:06 <asn> nickm: i did not even remember this was the case
17:11:17 <nickm> It was long ago
17:11:27 <nickm> we sent the current time in the introduce cell, so that cells could expire
17:11:47 <nickm> and so the replay cache entries could expire too
17:12:01 <nickm> problem is, the user's view of the current time is a fingerprint
17:12:12 <asn> yes, plus lots of issues with skewed clocks etc.
17:12:27 <asn> feel like anything that deals with time synchronization over the network is a recipe for trouble
17:12:46 <ahf> is the possible solution discussion something that makes more sense for #tor-dev afterwards? or does it make most sense now?
17:12:46 <mikeperry> nickm: 0ms delay between the packet that was just sent and tthe padding packet we want to send
17:12:59 <asn> ahf: i think it does not make most sense now
17:13:05 <asn> but it has been a useful discussion nevertheless
17:13:06 <nickm> asn, ahf: agreed
17:13:12 <nickm> mikeperry: ok
17:13:18 <ahf> cool, i don't want to interrupt it, but it sounds like something that can be taken outside of the meeting
17:13:28 <ahf> great, anything else for the kanban planning?
17:13:33 <asn> (im leaving after the meeting fwiw, so we would need to do it at another time)
17:13:42 <ahf> otherwise we will move to reviewer assignments
17:13:42 <nickm> asn: ok w me
17:13:49 <ahf> there is a very long link on the pad where you can see the list
17:14:06 <ahf> is everybody OK with the size of this list for the week?
17:14:12 <nickm> I'm ok
17:14:18 <asn> i think everyone took one. maybe me and nickm took two.
17:14:29 <asn> i gave mikeperry none because of the ESR review, but not sure about that
17:14:35 <ahf> i see dgoulet have two tickets there. should we move them to someone else?
17:14:38 <asn> hm
17:14:47 <nickm> (IMO yes, especially if they are for 042)
17:14:47 <ahf> both are hs related though
17:14:57 <mikeperry> 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 <asn> mikeperry: nope
17:15:16 <asn> so wrt dgouelt stuff:
17:15:27 <ahf> i can takeover something non-hs from someone who is comfortable reviewing hs related code
17:15:28 <asn> #25568 has been going on for a while. i dont think we should change that. last update was 5 weeks ago.
17:15:39 <asn> i can take #31652
17:15:59 <ahf> it is not too much then asn?
17:16:06 <nickm> (If you don't think we should change something, maybe move it to 043 or unspecified?)
17:16:20 <nickm> oh, it _is_ unspecified. Great
17:16:33 <asn> feel free to take #31657 if u want. i think it's super small anyway
17:16:37 <ahf> #25568 is from neel it seems
17:16:43 <asn> i will assign #31652 to me. it's also from neel.
17:16:46 <ahf> it is mergey_ready already?
17:16:50 <asn> i will let #25568 to dgoulet
17:17:09 <nickm> #31657 appears to be already merged to master, and pending a backport
17:17:10 <gaba> #31652 is the only one that really needs to be taking from dgoulet now
17:17:18 <ahf> #31652 is the one we care for?
17:17:21 <ahf> yeah
17:17:28 <asn> i took #31652
17:17:35 <gaba> ok
17:17:39 <ahf> ah, great
17:17:42 <ahf> thanks asn!
17:18:02 <ahf> and you wanted to get rid of #31657 the backport part?
17:18:04 <ahf> i can take that
17:18:13 <asn> sure but it's also fine
17:18:16 <asn> i think it's a trivial thing
17:18:27 <asn> i dont see #31657 mentioning backports or anything
17:18:30 <asn> it seems to be a refactoring thing
17:18:41 <ahf> ah, ack
17:19:12 <ahf> ok, next thing is to look at the 042 status page
17:19:23 <ahf> https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTorReleases/042Status
17:19:40 <ahf> this is both one of our weekly tasks, but also the first discussion point
17:19:49 <asn> i took a regression ticket that was unassigned there (#31408)
17:20:09 <ahf> 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 <nickm> we only have a few "must" tickets, but a bunch of "should"s
17:20:30 <ahf> 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 <ahf> yeah, must tickets have priority here
17:21:05 <nickm> I've tried to make all the "must" tickets "very high" priority, but I could have missed one or two
17:21:08 <ahf> 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 <nickm> (-should can also get downgraded to -can)
17:21:36 <ahf> #31091 sounds like something i should take
17:23:02 * ahf takes #10416 too
17:23:12 <ahf> i have the name and the OS for it
17:23:20 <nickm> :)
17:24:03 <nickm> I can  try #31107  if nobody else want it
17:25:06 <ahf> i am not gonna take #31022 yet, but i think i will take it in the future
17:25:13 <ahf> it seems low priority for me
17:25:19 <nickm> ack.
17:25:46 <nickm> Does anybody want to take #31483 from dgoulet? It seems important
17:26:08 <nickm> I guess I can do that, if you can review asn. (You reviewed the original single-counter-bucket code, right?)
17:26:25 <ahf> #31483 is not in this list yet?
17:26:28 <asn> hm
17:26:37 <asn> i dont know much about the counter bucket internals
17:26:42 <asn> i reviewed #15516 tho
17:26:44 <nickm> #31483 is not in the list of unassigned stuff, since it is assigned to dgoulet
17:26:45 <asn> i can review yes
17:26:52 <nickm> ok
17:26:53 <ahf> ah, right
17:28:38 <ahf> ok, cool. only non high priority tickets left now
17:29:03 <ahf> have everybody gone over and checked if any of the tickets that are should right now should be upgraded to -must?
17:30:09 <ahf> i take the silence as yes
17:30:10 <catalyst> i see only #31611 as "must"?
17:30:28 * ahf looks
17:30:47 <catalyst> i can help with #31611 but it could take lots of effort. does it need to be "must"?
17:30:53 <ahf> catalyst: we want to upgrade it to must?
17:31:11 <ahf> it is s31 must, but 042-should
17:31:27 <catalyst> ahf: oh, i see
17:31:53 <catalyst> 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 <ahf> 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 <catalyst> ahf: yea
17:32:36 <nickm> I don't think it needs to be must
17:32:38 <ahf> cool, let's have it mind when we do the next s31 planning session
17:33:03 <ahf> anybody mind if we move to the next item?
17:33:14 <ahf> it is the PR requirement discussion on network-team@
17:33:43 <ahf> 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 <ahf> one of the concerns that have been expressed is around the "avoid large PR's" requirement
17:34:26 <gaba> we could postpone that PR conversation until dgoulet is back?
17:34:44 <gaba> or not...
17:34:52 <ahf> 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 <ahf> 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 <ahf> so it doesnøt get put on someone desk very late in the process
17:35:28 <ahf> doesn't*
17:35:49 <asn> agreed
17:36:00 <ahf> 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 <gaba> it makes sense
17:36:27 <ahf> any objections/thoughts/ideas to all of this? :)
17:37:10 <asn> if it's impossible to get 100% follow rate, what is the policy gonna dictate?
17:37:19 <asn> aka what do we do with the PRs that dont follow the policy?
17:37:21 <gaba> good practice?
17:37:22 <catalyst> i don't have strong objections to most of the proposals, but could take another look
17:37:36 <gaba> expectations on what a PR should be
17:37:37 <asn> (im worried in particular about volunteer PRs etc.)
17:37:40 <gaba> and a review
17:37:41 <mikeperry> 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 <catalyst> at some level the reviewers and authors collectively enforce this
17:38:11 <nickm> asn: I think that we shouldn't enforce all this on volunteers when not necessary
17:38:22 <catalyst> 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 <nickm> mikeperry: we have ahd really nasty times with huge PRs in the past, fwiw
17:38:32 <ahf> 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 <ahf> 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 <catalyst> 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 <asn> ok sounds good
17:39:48 <ahf> catalyst: +1
17:39:49 <mikeperry> 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 <ahf> 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 <mikeperry> 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 <nickm> mikeperry: I think the idea is that the dependencies should not grow that large
17:41:59 <nickm> things should be merged piece by piece
17:42:21 <nickm> If it is too big to handle as 10 PRs, it is probably too big to handle as 1 PR
17:42:41 <catalyst> nickm: +1
17:42:48 <mikeperry> I am not seeing how that necessarily means 10 PRs is the solution?
17:42:51 <ahf> 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 <catalyst> 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 <nickm> mikeperry: also nobody is saying "do more PRs for the sake of it"
17:43:17 <ahf> 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 <mikeperry> 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 <nickm> 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 <catalyst> 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 <ahf> 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 <ahf> the thing just grows while being reviewed and then at the end it is passed to the mergers?
17:46:04 <mikeperry> 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 <nickm> Multiple smaller chunks merged over time also gives the mergers opportunity to notice problems and request changs early
17:46:17 <catalyst> ahf: i would prefer smaller PRs, with the prelim review being the earlier PRs in a series
17:46:31 <ahf> mikeperry: right
17:46:39 <catalyst> ahf: but sometimes we are in a situation where we already have large complicated PRs in progress
17:46:46 <mikeperry> 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 <ahf> yeah
17:47:29 <ahf> hm
17:47:38 <catalyst> 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 <catalyst> yes it can seem like more work overall
17:48:21 <ahf> 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 <mikeperry> 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 <ahf> is it more work for the sum of the team members or more work for the person working on this?
17:49:38 <mikeperry> I think both.. reviewers will have to review the refactoring tickets too
17:49:48 <nickm> mikeperry: the idea is not to have a bunch of branches all active at once,
17:49:56 <nickm> but to review and merge early and often.
17:50:12 <catalyst> 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 <mikeperry> 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 <catalyst> also we have less than 10 minutes remaining and i'm not sure we can arrive at consensus in that time
17:51:14 <mikeperry> isntead of everything near those line changes
17:51:20 <ahf> 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 <gaba> +1
17:52:03 <nickm> +1
17:52:12 <ahf> 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 <ahf> ok, we have some people who needs help with some stuff
17:52:40 <ahf> the first one is mike. i didnøt check if you were given reviews this week
17:52:46 <mikeperry> I can try.. I don't have a lot of spare time this month...
17:52:50 <ahf> but you would like not to have any reviews right now?
17:53:38 <gaba> yes, asn mentioned earlier that no reviews for mike this week
17:53:43 <ahf> great
17:53:44 <ahf> cool
17:54:18 <ahf> catalyst have some items related to CI
17:54:41 <catalyst> ahf: i've mostly updated the CI wiki page accordingly. maybe one or two tickets need opening
17:55:08 <ahf> oki, the jenkins failures are not something that needs help with this week?
17:55:42 <catalyst> ahf: not at the moment. i was leaving it there because teor made some comments
17:55:51 <ahf> ah, great
17:56:13 <ahf> swati: hey!
17:56:21 <ahf> swati: you need help with configure options and getting a trac account?
17:56:40 <swati> Hey ahf!
17:56:50 <ahf> 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 <ahf> i wonder why trac registration is closed right now though
17:57:07 <ahf> gaba: you know that?
17:57:07 <swati> yes, I need an account to create a pull requests.
17:57:16 <swati> pull requests.
17:57:19 <gaba> no but I'm creating a ticket
17:57:23 <gaba> mmm
17:57:29 <gaba> swati: you need an account in trac?
17:57:36 <gaba> PRs are made in github
17:57:37 <swati> yes.
17:57:50 <gaba> about trac, I'm creating a ticket for your account.
17:57:56 <ahf> thanks gaba
17:58:04 <ahf> okay, do we have anything else for the meeting?
17:58:11 <swati> 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 <ahf> or have i forgotten something important we have to talk about?
17:58:14 <swati> That's why asked.
17:58:18 <gaba> ahh, perfect.
17:58:22 <gaba> yes, thanks for asking!
17:58:36 <swati> Thanks, Gaba!
17:58:49 <swati> I will ask other questions on tor-dev.
17:59:13 <ahf> cool, i am gonna end the meeting now then
17:59:16 <ahf> #endmeeting