[19:59:33] New review: Krinkle; "bump" [integration/jenkins] (master); V: 0 C: 0; - https://gerrit.wikimedia.org/r/38645 [20:04:14] New patchset: Stefan.petrea; "Adding new pageview reports mobile (in progress)" [analytics/wikistats] (master) - https://gerrit.wikimedia.org/r/41979 [23:02:08] ^demon|lunch: so this branch merge bug in gerrit, does it happen every time? [23:02:25] <^demon|dinner> No, just some times. [23:02:52] <^demon|dinner> Lemme find the upstream bug. [23:02:58] it's ok, you can go have your dinner [23:04:04] <^demon|dinner> https://code.google.com/p/gerrit/issues/detail?id=1107 is related. [23:04:21] <^demon|dinner> We also hit https://code.google.com/p/gerrit/issues/detail?id=600 from time to time. [23:07:17] anomie: can you show me the git log -10 output from your local scribunto branch? [23:07:38] TimStarling- Just a minute [23:11:14] AaronSchulz: have you seen https://bugzilla.wikimedia.org/show_bug.cgi?id=43341 ? [23:14:15] TimStarling- https://en.wikipedia.org/w/index.php?title=User:Anomie/Sandbox&oldid=530996053 [23:18:20] ok, here's what I would suggest: [23:18:59] git branch ustring-v2 7cd8c69b651c9695ef880ada5f1826deb84c4bf5 [23:19:09] git checkout ustring-v2 [23:19:18] git cherry-pick 64b522d69b4603d6cfe01f24a5d1cad47406a22e [23:19:21] git commit --amend [23:19:26] (remove change-id) [23:19:43] git cherry-pick 51e113f4e5f1acc4c6718e3610c2356ad7830c4c [23:19:49] git commit --amend [23:19:55] git cherry-pick 8de5559fb8d9a828f5de13d521b22469359dc21b [23:19:58] git commit --amend [23:20:00] git review [23:21:02] then we can abandon the equivalent changes that you submitted against master since they will be superseded by the branch merge [23:22:25] (for all three git commit --amend commands above, you remove the change-id) [23:26:09] error: Commit 51e113f4e5f1acc4c6718e3610c2356ad7830c4c is a merge but no -m option was given. [23:26:09] fatal: cherry-pick failed [23:26:46] how many commits are there in those branches? [23:28:07] There are 10 commits in all, plus two merges in there. [23:28:34] maybe use git merge --squash [23:28:38] Maybe I should just rebase them all in semi-arbitrary order. [23:28:47] Or that, if we don't mind one huge commit. [23:28:50] if you don't mind them being squashed [23:29:06] then you will get one change in gerrit per branch merge [23:29:39] I haven't tried it myself, I usually don't make such a mess of my branches ;) [23:30:14] Well, I started on the ustring stuff, then I had to fix various bugs, then I split out some of the changes for easier review, and it wound up in a know [23:30:15] knot [23:31:15] --squash doesn't commit [23:31:27] so it will be: [23:31:43] git merge --squash fix-module-string-override && git commit [23:31:54] then you can enter a commit message describing the changes made in that branch [23:32:07] then the same for the other two branches [23:33:08] as for making things easy to review: things are easy to review when I can read the diffs [23:33:38] that's more important than having changes split up so that each commit does one thing only [23:34:18] and I'd rather have changes from two different projects interleaved than have them in two different branches [23:34:42] unless the branches are really huge, like tens of thousands of lines of code [23:36:01] even then, you don't want to be changing the same file in two different branches unless someone is forcing you to [23:36:10] that's about the worst thing that can happen, in any version control system [23:38:41] TimStarling- I've squashed it into 3 commits: almost-everything, language module, and ustring module. Sound good? [23:38:57] yes [23:39:27] TimStarling: no [23:39:30] then when you do "git review", it should ask if you want to submit maybe 6 commits [23:39:49] if so, the answer is yes [23:41:15] AaronSchulz: can I assign it to you? [23:41:30] so you disabled redirect purges? [23:41:41] TimStarling- Ok: https://gerrit.wikimedia.org/r/#/c/42049/ https://gerrit.wikimedia.org/r/#/c/42050/ https://gerrit.wikimedia.org/r/#/c/38013/ [23:42:00] no, years ago, I disabled template purges when there were more than 500 pages to purge [23:42:08] you disabled redirect purges, accidentally [23:42:16] regardless of the type of update? ok [23:42:31] where is the code to disable them? [23:42:38] MZ links to it [23:42:55] is this just a problem of interaction? [23:43:05] https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=blob;f=includes/job/jobs/HTMLCacheUpdateJob.php;h=55c9f991d9e3b731d463d057b841f30f5e9ab8c0;hb=refs/heads/wmf/1.21wmf6#l245 [23:43:28] basically [23:44:13] ideally I would like it to start purging again, except when the number of pages to purge is more than some limit [23:44:25] not quite sure how it should be implemented [23:44:54] maybe just re-enable purging in the case where the job is queued rather than run immediately from HTMLCacheUpdate [23:45:19] but ideally the limit would be separately configurable [23:45:36] the problem is an uncorrected design flaw that's been in the code for years [23:46:05] specifically, you could have a template which is used by a million pages [23:46:29] and with the original code, it would go ahead and happily purge them all from squid [23:46:36] and simultaneously remove them all from the parser cache [23:46:53] so you might suddenly drop the squid cache hit rate from 70% to 35% [23:47:08] and drop the parser cache hit rate by a similar amount [23:48:09] so after something like that happened once, I commented out that code in HTMLCacheUpdateJob.php [23:49:03] actually what would really be ideal is if the updates could be spread out over a few days [23:49:15] then we could have a current version of the template on every page that uses it, without crashing the site [23:49:35] but obviously that's more complicated still [23:49:59] yeah, I was thinking about stalling it out [23:50:35] but in a way that does not just sleep() or something stupid that wastes time [23:50:46] yeah, trickier [23:51:06] * AaronSchulz is a bit surprised that patch still applied though sees it on fenari [23:53:11] * AaronSchulz dislikes having a bunch of patches applied for wmf branches [23:53:18] it's not very transparent at all [23:55:49] Reedy: still awake? [23:56:06] Yup [23:56:26] Just had one of my internet connections die, just been trying to work out why [23:57:17] Reedy: I guess that patch should be removed [23:58:37] TimStarling: I guess some $wgMaxBacklinkedPagesInvalidate type variable is ok for now [23:59:12] Reedy: is that patchlist stuff still in svn? [23:59:24] AaronSchulz: should be fine [23:59:33] patchlist stuff? [23:59:38] what number? [23:59:39] It's in mediawiki/tools/release.git [23:59:40] Reedy: for wmf branches [23:59:46] It uses a git revision [23:59:57] Reedy: so I see 366 references to /usr/local in the apache configuration