Since this patch is failing in pre-merge testing at the moment, I'm planning on going with option #2: take the branch before the big commit that caused this bug in the first place. It has passed our usual pre-commit testing, so I'm going to clobber branch-2.8.0 in order to rewind it to
https://git-wip-us.apache.org/repos/asf?p=incubator-impala.git;a=commit;h=48792eb92f010af99246a14d5a3570dfed662dfd Keep in mind that if you want to test this with LZO, you need https://github.com/cloudera/impala-lzo/commit/41aeaa4ca1451419f52e8cffbd4264436a6f3db9 On Thu, Jan 5, 2017 at 2:14 PM, Tim Armstrong <[email protected]> wrote: > In any case, here's the CR for the bugfix > https://gerrit.cloudera.org/#/c/5615/ > > On Thu, Jan 5, 2017 at 1:10 PM, Henry Robinson <[email protected]> wrote: > >> Same deal with me - I don't support reverting in master. >> >> I think it's fine to branch from before that change and cherry pick what >> you can. As the RM you decide what goes in and what doesn't - if the cherry >> picks are too hard, it's fine to skip them. Patching is also fine. >> >> On Thu, Jan 5, 2017 at 12:12 PM Tim Armstrong <[email protected]> >> wrote: >> >> > Oh yes my reading comprehension was bad. I don't think it makes sense to >> > >> > revert it on master - I thought you meant reverting it on the branch. >> > >> > >> > >> > The bugfix is small and straightforward - maybe it's easiest if I just >> put >> > >> > that together and put it up for review. >> > >> > >> > >> > - Tim >> > >> > >> > >> > On Thu, Jan 5, 2017 at 12:02 PM, Jim Apple <[email protected]> wrote: >> > >> > >> > >> > > Just to clarify: when I said reverting it, I meant reverting it in >> > >> > > master, too, then chery picking that change to the branch. I'd rather >> > >> > > keep the branch free from as many non-master commits as possible. >> > >> > > >> > >> > > On Thu, Jan 5, 2017 at 11:55 AM, Tim Armstrong < >> [email protected]> >> > >> > > wrote: >> > >> > > > +1 for reverting it. It doesn't add any new functionality so I don't >> > see >> > >> > > > the value in including it in the release. >> > >> > > > >> > >> > > > On Thu, Jan 5, 2017 at 10:58 AM, Henry Robinson <[email protected]> >> > >> > > wrote: >> > >> > > > >> > >> > > >> +1 for reverting it. It's a recent, major change and it's still >> > >> > > settling. >> > >> > > >> >> > >> > > >> On 5 January 2017 at 10:49, Jim Apple <[email protected]> wrote: >> > >> > > >> >> > >> > > >> > Yes, that is in the branch: >> > >> > > >> > https://git-wip-us.apache.org/repos/asf?p=incubator-impala. >> > >> > > >> > git;a=shortlog;h=refs/heads/branch-2.8.0 >> > >> > > >> > >> > >> > > >> > Here are some options: >> > >> > > >> > >> > >> > > >> > 1. Burn this branch, make a new one without the commit but with >> > >> > > >> > everything else. Pros: no blocker. Cons: cherry-picking hell. >> > >> > > >> > >> > >> > > >> > 2. Take the branch before this commit. Pros: no blocker. Cons: >> > missing >> > >> > > >> > other bug fixes >> > >> > > >> > >> > >> > > >> > 3. Wait for a fix. Pros: no blocker. Cons: delay >> > >> > > >> > >> > >> > > >> > 4. Commit to master a git revert of that patch. Pros: no blocker; >> > >> > > >> > fixes blocker on branch and master. Cons: add noise to commit >> > history >> > >> > > >> > >> > >> > > >> > I'd like to git revert it. What do you all think? >> > >> > > >> > >> > >> > > >> > On Thu, Jan 5, 2017 at 10:39 AM, Tim Armstrong < >> > >> > > [email protected]> >> > >> > > >> > wrote: >> > >> > > >> > > This one: >> > >> > > >> > > >> > >> > > >> > > http://gerrit.cloudera.org:8080/4418 >> > >> > > >> > > >> > >> > > >> > > On 5 Jan 2017 10:15 AM, "Jim Apple" <[email protected]> >> wrote: >> > >> > > >> > > >> > >> > > >> > >> Which commit introduced it? >> > >> > > >> > >> >> > >> > > >> > >> On Thu, Jan 5, 2017 at 10:12 AM, Tim Armstrong < >> > >> > > >> [email protected] >> > >> > > >> > > >> > >> > > >> > >> wrote: >> > >> > > >> > >> > I think we have some open blockers for 2.8. Or at least one >> > that >> > >> > > was >> > >> > > >> > >> > introduced in a recent commit . >> > >> > > >> > >> > https://issues.cloudera.org/browse/IMPALA-4707. Do we plan >> to >> > >> > > >> > include a >> > >> > > >> > >> fix >> > >> > > >> > >> > or just exclude the commit that introduced it? >> > >> > > >> > >> > >> > >> > > >> > >> > On 5 Jan 2017 9:09 AM, "Jim Apple" <[email protected]> >> > wrote: >> > >> > > >> > >> > >> > >> > > >> > >> > I have now also tested the docs build: >> > >> > > >> > >> > http://jenkins.impala.io:8080/view/Utility/job/docs-build/ >> 92/ >> > >> > > >> > >> > >> > >> > > >> > >> > On Thu, Jan 5, 2017 at 8:28 AM, Jim Apple < >> > [email protected]> >> > >> > > >> > wrote: >> > >> > > >> > >> >> I have now tested this hash (4fa9270e647b9c097295dcc13d9713 >> > >> > > >> > 6cca3139ad) >> > >> > > >> > >> >> on public Jenkins: >> > >> > > >> > >> >> >> > >> > > >> > >> >> http://jenkins.impala.io:8080/ >> view/Utility/job/parallel-all- >> > >> > > >> > tests/130/ >> > >> > > >> > >> >> http://jenkins.impala.io:8080/ >> view/Utility/job/ubuntu-14.04- >> > >> > > >> > >> > from-scratch/434/ >> > >> > > >> > >> >> http://jenkins.impala.io:8080/ >> view/Utility/job/ubuntu-14.04- >> > >> > > >> > >> > from-scratch/435/ >> > >> > > >> > >> >> http://jenkins.impala.io:8080/ >> view/Utility/job/ubuntu-14.04- >> > >> > > >> > >> > from-scratch/436/ >> > >> > > >> > >> >> >> > >> > > >> > >> >> That covers RAT (the tool for checking copyright >> compliance), >> > >> > > >> various >> > >> > > >> > >> >> build options (including ninja, release, asan, shared libs), >> > >> > > >> loading >> > >> > > >> > >> >> the data from scratch and running all tests in core and in >> > >> > > >> > exhaustive, >> > >> > > >> > >> >> clang-tidy, and the build we instruct IPMC release testers >> to >> > >> > > run >> > >> > > >> > >> >> (bin/bootstrap_build.sh). >> > >> > > >> > >> >> >> > >> > > >> > >> >> I have also created a git branch: >> > >> > > >> > >> >> https://git-wip-us.apache.org/repos/asf?p=incubator-impala. >> > >> > > >> > >> > git;a=shortlog;h=refs/heads/branch-2.8.0 >> > >> > > >> > >> >> >> > >> > > >> > >> >> I am working on a commit to add a disclaimer to the docs >> > >> > > >> > >> >> (https://gerrit.cloudera.org/#/c/5610/) and then I will >> > upload >> > >> > > a >> > >> > > >> > >> >> release candidate tarball. >> > >> > > >> > >> >> >> > >> > > >> > >> >> Please prepare yourself to vote. Instructions are here: >> > >> > > >> > >> >> >> > >> > > >> > >> >> https://cwiki.apache.org/confluence/display/IMPALA/ >> > >> > > >> > >> > DRAFT%3A+How+to+Release#DRAFT:HowtoRelease- >> > >> > > >> > HowtoVoteonaReleaseCandidate >> > >> > > >> > >> >> >> > >> > > >> > >> >> On Wed, Jan 4, 2017 at 7:17 PM, Jim Apple < >> > [email protected] >> > >> > > > >> > >> > > >> > wrote: >> > >> > > >> > >> >>>> I'd figure out a way to add a big caveat to the docs. >> Maybe >> > on >> > >> > > >> the >> > >> > > >> > >> > landing >> > >> > > >> > >> >>>> page? Even better if there's a template we can add a >> caveat >> > to >> > >> > > >> that >> > >> > > >> > >> > appears >> > >> > > >> > >> >>>> on every page. >> > >> > > >> > >> >>> >> > >> > > >> > >> >>> I like this idea. I'll prepare a patch for the landing >> page. >> > >> > > >> > >> >>> >> > >> > > >> > >> >>> I don't think there is a simple way to do it on every page. >> > >> > > John, >> > >> > > >> > >> >>> Laurel, am I wrong abut that? >> > >> > > >> > >> >> > >> > > >> > >> > >> > > >> >> > >> > > >> >> > >> > > >> >> > >> > > >> -- >> > >> > > >> Henry Robinson >> > >> > > >> Software Engineer >> > >> > > >> Cloudera >> > >> > > >> 415-994-6679 >> > >> > > >> >> > >> > > >> > >> > >>
