[04:28:50] --- Derrick Brashear has become available [04:29:16] well. whenever this meeting is. but anyway, there, reviewed more patches. now blind. [04:42:06] --- Derrick Brashear has left: Disconnected [05:19:23] --- Marc Dionne has become available [05:28:01] --- Derrick Brashear has become available [05:30:53] --- meffie has become available [05:39:37] --- meffie has left [06:07:32] --- meffie has become available [06:13:39] so. isn't it whatever time UTC that we normally met, already? or is my time zone math bad? [06:14:55] time.gov is showing ... 13:14. [06:15:08] Which would mean ... we have almost two hours left? [06:15:22] yeah, ok. so it's the wrong way. we're meeting late not early. fine. [06:15:27] (i just saw the email) [06:15:56] Math is hard, apparently. (I thought it was 9 eastern, too.) [06:17:06] oh. spring forward, so 10 becomes 11, and we resync when they move forward. duh. [06:18:31] Gives me some time to actually look at gerrit changes, at least. [06:25:44] --- Derrick Brashear has left: Disconnected [06:32:45] --- Marc Dionne has left [06:32:47] --- Marc Dionne has become available [06:54:51] --- squinney has become available [06:58:31] --- deason has become available [07:14:18] --- Derrick Brashear has become available [07:14:52] --- Simon Wilkinson has become available [07:15:34] --- Simon Wilkinson has left [07:15:55] --- Simon Wilkinson has become available [07:57:10] nobody happens to know the gerrit search string for changes that don't have a code review from a specific user, do they? [07:57:24] the docs seem to say it exists, but don't give an example [07:59:43] --- Stephan Wiesand has become available [08:00:12] --- Marc Dionne has left [08:00:13] http://gerrit.openafs.org/#q,-reviewer:1000008,n,z [08:00:38] --- Marc Dionne has become available [08:01:02] handy [08:01:17] that's the link for andrew. you need to figure out your own userid of course [08:01:36] keywords here: [08:01:37] https://gerrit.wikimedia.org/r/Documentation/user-search.html [08:01:38] doesn't that also exclude things I'm just a reviewer on? [08:02:32] (and according to that page, reviewer:self gives you yourself without needing to know your id) [08:02:38] confession: I drowned in top priority local work since the last meeting and had no time for openafs :-( [08:02:40] I was thinking of the label:CodeReview thing [08:02:43] you can use names and not ids [08:03:25] --- paul.smeddle has become available [08:03:37] I'm not sure that "self" works in the version of gerrit we have deployed [08:03:54] Hello, sorry for my lateness [08:03:58] http://gerrit.openafs.org/#q,-reviewer:jaltman,n,z certainly works [08:04:03] --- Simon Wilkinson has left [08:04:03] Hello. [08:04:59] but it doesn't state whether or not a review has been issued by that user, just that the user is listed as a reviewer which could have been added by someone else or because they own the commit [08:05:46] true. [08:06:15] at 5 minutes a pullup a person will require at least 12 hours to review them all [08:06:36] --- Simon Wilkinson has become available [08:06:43] 5 minutes a pullup is a long time for many of those [08:06:45] It seems the coverity patches are getting some reviews (I looked at them this morning), but shall we start by looking at the patches we need/want for 1.6.2.1 ? [08:07:44] my point is that it is unreasonable to assume that the majority of the release-team had that amount of time to dedicate to openafs over the weekend or over the last three days. I certainly have not. [08:08:25] we can still discuss general issues, and the small number of patches for "1.6.2.1" [08:08:55] Wasn't 1.6.2.1 settled in the last meeting? [08:10:01] Merging the patches for that, agreed in the last meeting, is clearly next I think. [08:10:11] I didn't think there was anything to discuss regarding 1.6.2.1 other than getting the patches, release notes, NEWS, versions, etc. onto the branch [08:10:46] NEWS also still lacks the last entry from the release notes. [08:11:28] that should be added [08:12:14] I'm planning to get 1.6.2.1 test tarballs ready tomorrow. Can add that NEWS entry too once at it. [08:12:40] thanks [08:14:04] well, so is a 1.6.2.1 actually existing? from the last meeting, I thought we ended on "we'll pull the patches as if 1.6.2.1 exists, but we haven't decided if it will actually exist" [08:14:13] Was there not a question mark around 8948 ? [08:14:26] i.e. Ken was seeing an issue ... [08:14:35] I don't think that blocks inclusion [08:14:52] since it's not _caused_ by that patch, if I understand it correctly [08:14:54] okay, sure, just thought I'd mention it [08:16:26] Regarding the coverity fixes, we also had an agreement. If they lack review, Paul & me should "invite" folks. [08:17:41] Looking at the rest of what's queued in gerrit, my feeling is that it's too much for one release to include. [08:20:19] I agree [08:20:19] what would you prefer? maybe leaving out anything remotely "feature"-like, and do the simpler bugfix and small stuff for the next release? [08:20:19] I had the same initial reaction so I decided to look at the number of patchsets included in all releases from 1.4.0 to present. The numbers range from ~90 to ~250 patchsets per release. I don't think the number of patchsets is necessarily a blocker. However, I do think that given the number of patchsets that are desired it may take more than two months to perform proper review and testing. [08:21:18] It seems like the coverity patches are small and "hardly count" for some of these calculations. [08:21:22] yes, and along those lines, more frequent, smaller releases may get the patches out to the community on the same timeframes, but in safer, bite-sized chunks ... [08:21:33] Obviously they take release engineer time, of course. [08:21:46] I think the question for Paul and Stephan to consider as release managers is whether they want to have a larger number of more frequent releases with smaller set of patches or a smaller number of updates with longer testing time for pre-releases. [08:22:00] 95% of the coverity patches were trivialish off-by-one errors, so easy to review ... [08:22:27] paul: yes, those are not the problem. [08:23:30] I wouldn't say they "don't count"; they're similar to a lot of the other patches, and no necessarily more "safe"; one of them already had an error fixed by a subsequent one, iirc [08:23:34] My personal preference is smaller releases on a 2-3 month cadence BUT that may be unsupportable in terms of man hours (testing, building, packaging) [08:23:49] the trade off as I see it primarily has to do with release cycle vs upgrade patterns at organizations. the majority of orgs are only able to upgrade once or twice a year [08:26:05] If we release every 2-3 months, sites will skip releases. But I don't think that's a problem. [08:26:32] I agree with Andrew, the coverity patches need to be reviewed. the majority of the patches that go onto master do not get nearly enough review in my opinion. pullups to the stable branch must take reviews seriously [08:28:15] Sure. Many of them are easy to review though, compared to other changes I've seen go into 1.6.2. [08:29:01] I just had no time yet to even look briefly at all the others. Project finished 5 minutes before the meeting... [08:30:20] We're never going to have a release schedule that meets every sites upgrade schedule. The advantage of frequent releases is that they will be less out of date, than they would if they just miss a yearly release [08:30:41] simon: yes. [08:30:57] Maybe we should discuss today those changes that someone thinks are really really important? [08:31:26] I think anything that smells like a feature could safely be delayed [08:32:12] Unless it's very trivial and extremely useful, yes. [08:32:46] possibly "important": 7705, 9587 (in some form), 9588 [08:33:49] I just +1'd the pullup of 7705... [08:34:13] 7705 I didn't expect to be controversial or anything [08:34:34] 9587 is a question of "what to do for 1.6.3", and I'm fine with "just add /etc/krb5/krb5.conf to the search path" [08:35:05] 9588 I think is a matter of if we block on afs3-stds &c discussion [08:35:11] and if that limit (5000) is okay [08:36:12] --- Simon Wilkinson has left [08:36:32] --- Simon Wilkinson has become available [08:38:18] Well, it sounds like Jeffrey wants mailing list discussion for 9588; why don't we pick a venue and assign someone to start the thread? [08:38:45] yes, that will happen [08:39:06] I just thought we should be agreed that we shouldn't block on that if it turns out to take a while or something [08:39:32] (er, if we are agreed) [08:39:55] Hmm, these are all still available for master only. SOme are not even merged. Those are not for 1.6.3 IMHO. [08:39:58] Are you volunteering to start the email thread? [08:40:49] 9362. just merge it. it's a typo in a header. [08:41:00] So, why can't we just take 9587 as is? [08:41:19] kaduk: yes [08:41:24] 9595. ony touches openbsd. [08:41:37] derrick: is that important? there's tons of little fixes; we're not going through all of them right now [08:42:00] 9362? until it's fixed, external users of the api don't work :\ [08:42:03] paul.smeddle: derrick said some people do use the krb5-weak.conf thing; that patch as-is removes the functionality [08:42:15] ah, right. [08:43:15] well, external to src/util. [08:43:18] Stephan Wiesand: well, some are things we didn't discovery until 1.6.2 [08:43:24] er, discover [08:43:53] andrew, are those regressions against 1.6.1? [08:44:40] regressions should of course be fixed. if the problem was there since 1.6.0, the fix can wait for 1.6.4. [08:46:22] While not a hard-and-fast rule, I agree that's a good heuristic [08:46:38] I'm not sure when the krb5-weak.conf whatever was put in.... [08:46:55] er, for 9587 [08:47:37] I consider that a regression [08:47:40] but right now it's effectively "aklog doesn't work on sol 11", and the 1.6-specific fix will be changing a string [08:47:51] an env var, that is [08:48:26] 9588 is not a regression, but it is related to the security stuff [08:50:39] 9588 seems controversial though? [08:50:59] --- Simon Wilkinson has left [08:51:16] as far as I know, the only controversial aspects are what specifically to do; I'm not sure if anyone is advocating delaying it for another release [08:51:33] (and yes, I'll make a thread today) [08:52:04] --- Simon Wilkinson has become available [08:52:12] I do not consider 9588 a must-have for 1.6.3 [08:52:18] there is also a pretty damn annoying regression in vos; change.... [08:52:39] (ah, okay) [08:52:43] I don't think we should be considering major changes for 1.6 that haven't bedded in on master first [08:53:07] what's the vos regression? [08:53:12] change... apparently there is no change for it, because the person hasn't submitted it yet for some reason [08:53:17] arg [08:53:37] you can still tell us what it is [08:53:55] the vsprocs "DoVolDelete" refactoring makes "vos move" and possibly some other operations print out error messages when there is no error [08:54:20] sorry, I don't have specifics in front of me, and there's a patch somewhere for it... [08:54:50] ok, sounds reasonable for inclusion in principle [08:54:54] but from what I remember we try to delete something that in the common case is not there (e.g. the destination volume for a move), and we get VNOVOL, and print out the error with afs_print_error or whatever [08:55:51] the options are of course simply trying to fix it, or backing-out the change like we did for 1.6.1 [08:56:20] (and re: 9588, okay, I won't push for that _now_, unless someone else has opinions for it) [08:56:32] having nothing to do with the release process. But I consider it courteous for someone that finds a bug in another contributor's code to notify the author of the change. [08:56:51] (i poked the person that has the patch) [08:56:57] --- Simon Wilkinson has left [08:57:10] and give that contributor the opportunity to right the wrong [08:57:12] --- Simon Wilkinson has become available [08:57:52] it needs to be fixed on master regardless. [09:01:21] that can be reviewed after the change actually hits; sorry, I thought that was submitted to master [09:01:53] are we looking for other "important" things worthy of discussion, then? [09:03:01] yeah, anything in the glut of 1_6_x patches for review that isn't either mentioned in last week's minutes or today yet [09:04:43] I'm not sure of the impact of 9492; it seems like without that, we're missing cache checks [09:06:47] oh, and 9279 I would push for, but I think that's already been discussed and people are familiar with it [09:09:26] I'd be ok with those. [09:09:49] --- Simon Wilkinson has left [09:09:58] --- Simon Wilkinson has become available [09:10:55] is there agreement that we're delaying anything remotely feature-like? should I (or someone) -2 feature-like changes to mark them as separate from the rest? [09:11:05] I'm fine with both of those. the equivalent of 9279 was included in 1.7.22 [09:11:44] jeff: thanks [09:11:49] I agree with 9279 in principle but don't think I've reviewed the locking. (I vaguely recall Simon assuring me that it was okay, though.) [09:12:18] andrew: -1 should suffice ;-) [09:13:03] the issue with locking is that by dropping the lock and reacquiring it alters timing between threads that can adversely impact performance. in this case, the call is dead so it doesn't matter as much if it is dead faster or slower. [09:13:18] well, I wasn't sure if that would be confusing if someone -1'd a change for, like, actually objecting to it [09:13:57] I prefer -2 for things that should be put off. Things that should not be on 1.6.x at all can be abandoned [09:14:01] ideally we could do something to remove deferred changes from the list of changes we're looking at for 1.6.3 [09:14:10] 9279 is fine - there's no worry about it changing thread timings as its such a rare case [09:14:23] -2 seems easy enough to filter out, and not likely to be on something we're considering for 1.6.3 [09:14:33] seems reasonable [09:14:40] okok... [09:15:13] We may want a feature or two in a release though as bait. [09:16:13] (how intrusive is the addition of fs flushall?) [09:17:04] it is a change to the cache manager. [09:17:27] I think that's on the side of "less likely to break stuff" as far as features go [09:18:08] from what I recall, the code is very very close to "flushvolume but without checking the volid" [09:18:27] since flushvolume already iterates over like everything, and just does "if (fid.volid == volid)" [09:18:46] yes, that is correct. [09:18:52] Please don't -2 those related to flushall ;-) [09:19:15] I think we indicated last week that we were keen on that feature [09:19:33] then perhaps you should +2 it [09:20:08] also, if we want the list of changes to maybe be a bit smaller, those doc updates could probably go in [09:20:17] i wish we had more people running it before it goes in a "point" release. [09:20:22] I don't think those need "as much" review as others [09:20:35] jeff, yes, but not without consulting with the experts, no matter how much I like it [09:21:59] Paul, I think we can just merge doc updates that seem reasonable? [09:22:03] as a reminder, nothing should get merged until after 1.6.2.1 is prepared [09:22:03] should I maybe +2 things that are doc updates, or just change constant strings in error messages? (I think those are trivial enough that they could just be included) [09:22:07] so they are flagged [09:22:10] and yes, yes, after 1.6.2.1 [09:22:30] just +2 things with a message "approved for 1.6.3" [09:24:51] stephan, yes, I think so [09:26:11] (I'd like an "ok" from paul/stephen...) [09:26:32] deason: ok [09:26:34] :) [09:27:00] can i get an amen? [09:27:08] "ok" [09:27:13] oh man, just what I wanted [09:27:34] don't lose them ... [09:27:46] derrick: on what? [09:27:58] well, is there anything else for this meeting? [09:29:00] Andrew, no, I think we just have to get started. Sorry again for not working on 1.6.2.1 yet, as I had planned. [09:29:36] will there be a meeting next week? [09:29:52] We _will_ announce one with more warning next week. [09:30:04] Yes. [09:30:12] --- squinney has left [09:31:26] Great, thanks everyone. [09:32:57] --- meffie has left [09:33:26] Ok, thanks All. [09:33:29] --- Stephan Wiesand has left [09:37:32] --- Marc Dionne has left [09:37:41] --- Marc Dionne has become available [09:41:23] --- Marc Dionne has left [09:42:16] --- Marc Dionne has become available [09:46:40] --- paul.smeddle has left [09:52:24] --- Simon Wilkinson has left [09:57:15] --- Marc Dionne has left [09:58:08] --- Marc Dionne has become available [10:33:16] --- Derrick Brashear has left: Disconnected [10:42:17] --- Simon Wilkinson has become available [10:44:35] --- Simon Wilkinson has left [10:46:31] --- Simon Wilkinson has become available [10:47:28] --- Simon Wilkinson has left [10:52:09] --- Marc Dionne has left [10:52:33] --- Marc Dionne has become available [10:55:00] --- Simon Wilkinson has become available [10:56:11] --- Derrick Brashear has become available [12:03:53] --- Jeffrey Altman has left: Disconnected [12:04:53] --- Marc Dionne has left [12:05:35] --- Marc Dionne has become available [12:05:38] --- Jeffrey Altman has become available [12:23:49] --- Marc Dionne has left [13:20:31] --- ktdreyer has left [14:11:26] --- Derrick Brashear has left: Disconnected [14:13:39] --- Simon Wilkinson has left [16:06:00] --- deason has left [16:28:45] --- Derrick Brashear has become available [18:38:45] --- Derrick Brashear has left: Disconnected [19:38:24] --- Derrick Brashear has become available