You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Justin Ling Mao <ma...@sina.com> on 2021/02/10 09:26:27 UTC
Re: Re: [Commit Accident Case Study] Commit 4faf507 broke the build
Haha, it scared me. Let me go through this accident.
The root cause is: I'm over-confident, frivolous and hasty. I flatter myself that it's just a typo and committing it could not have anything bad happens. And I also don't give this PR a buffer time for other people's review.
Accident is bad, but it's much more terrible if we can not reflect on it and think about how to avoid it next time.
For remedy:
I will add a new section: Commit Accident Case Study in [1] for the successor’s learning (Can anyone give me the permission to edit that wiki)?
I will sum up our commit rules and the checklists before committing one patch, and do some works to use the Github CI and commit script to protect/check these constraint.
For punishment:
I will frozen/forbid my committership permission for three months(02-10 ~ 05-10). During this period, I must not commit anything. I wish I could reflect on my fault and have a better understanding on the wording: "With great power comes great responsibility"
Reference:[1] https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
----- Original Message -----
From: Andor Molnar <an...@apache.org>
To: maoling199210191@sina.com
Cc: dev <de...@zookeeper.apache.org>
Subject: Re: Commit 4faf507 broke the build
Date: 2021-02-10 00:26
I’m sorry Justin. There’s no excuse for a mistake like this. We should not show mercy for anybody, otherwise it would erode the trust in our community. Your committership is now revoked.
Just kidding. Don’t worry at all. ;-)
I reverted the patch, so now please create a new PR with all the required changes included.
Also I second Enrico’s comment: if CI is in bad shape, we should fix it.
Regards,
Andor
> On 2021. Feb 9., at 13:45, Justin Ling Mao <ma...@sina.com> wrote:
>
> Oops, it's my blame. I'm very sorry for my mistakes. Since these days the CI is in disorder and it's a typo, so I'm not waiting for CI check and forgot that an UT has covered this change although I wrote these related codes. It's all my mistake and I will summarize our submission process and this accident. I will write another letter to discuss the commit rules and how to improve our code review throughput
>
>
> ----- Original Message -----
> From: Andor Molnar <an...@apache.org>
> To: DevZooKeeper <de...@zookeeper.apache.org>
> Subject: Commit 4faf507 broke the build
> Date: 2021-02-09 19:43
>
> Hi,
> I noticed that the latest commit 4faf507 ZOOKEEPER-4007: A typo in the ZKUtil#validateFileInput method broke the build, because the unit test has not been amended.
> I reverted the commit to fix the build. Please create new PR with a proper patch.
> Has the committer verified that the build is green before submitting it?
> Andor
Re: [Commit Accident Case Study] Commit 4faf507 broke the build
Posted by Andor Molnar <an...@apache.org>.
Hi Ben,
Sure, here’re the daily builds:
https://ci-hadoop.apache.org/view/ZooKeeper/job/zookeeper-multi-branch-build/
I see bind failed (address already in use) exception much more frequently on branch-3.7 and master branches. It would worth to diff with 3.6, I suspect we introduced something recently which causes this.
Andor
> On 2021. Feb 12., at 20:01, Benjamin Reed <br...@apache.org> wrote:
>
> thank you for figuring this out! two questions:
>
> 1) is there a daily test build still running? if so, where can we see
> its status?
> 2) what is the easiest way to retrigger tests? (sorry, i know i've
> asked this before :'( )
>
> ben
>
> On Wed, Feb 10, 2021 at 2:18 AM Szalay-Bekő Máté
> <sz...@gmail.com> wrote:
>>
>>> For punishment:
>>> I will frozen/forbid my committership permission for three months
>>
>> I think you took this too seriously. Mistakes / accidents happen when
>> someone is working (I did much more serious ones myself on different
>> projects). And the community is grateful for the contribution, no one
>> should expect perfection. At least I hope so, for my sake :p
>>
>> Independently from this issue we really should focus on making our CI to be
>> rock-solid. So if the CI is red, then we could assume the PR broke
>> something. Currently I think flaky tests and independent CI issues are more
>> frequently causing red builds than actual failures introduced by PRs.
>>
>> Cheers,
>> Mate
>>
>> On Wed, Feb 10, 2021 at 10:26 AM Justin Ling Mao <ma...@sina.com>
>> wrote:
>>
>>> Haha, it scared me. Let me go through this accident.
>>> The root cause is: I'm over-confident, frivolous and hasty. I flatter
>>> myself that it's just a typo and committing it could not have anything bad
>>> happens. And I also don't give this PR a buffer time for other people's
>>> review.
>>> Accident is bad, but it's much more terrible if we can not reflect on it
>>> and think about how to avoid it next time.
>>> For remedy:
>>> I will add a new section: Commit Accident Case Study in [1] for the
>>> successor’s learning (Can anyone give me the permission to edit that wiki)?
>>> I will sum up our commit rules and the checklists before committing one
>>> patch, and do some works to use the Github CI and commit script to
>>> protect/check these constraint.
>>> For punishment:
>>> I will frozen/forbid my committership permission for three months(02-10 ~
>>> 05-10). During this period, I must not commit anything. I wish I could
>>> reflect on my fault and have a better understanding on the wording: "With
>>> great power comes great responsibility"
>>>
>>> Reference:[1]
>>> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
>>> ----- Original Message -----
>>> From: Andor Molnar <an...@apache.org>
>>> To: maoling199210191@sina.com
>>> Cc: dev <de...@zookeeper.apache.org>
>>> Subject: Re: Commit 4faf507 broke the build
>>> Date: 2021-02-10 00:26
>>>
>>> I’m sorry Justin. There’s no excuse for a mistake like this. We should not
>>> show mercy for anybody, otherwise it would erode the trust in our
>>> community. Your committership is now revoked.
>>> Just kidding. Don’t worry at all. ;-)
>>> I reverted the patch, so now please create a new PR with all the required
>>> changes included.
>>> Also I second Enrico’s comment: if CI is in bad shape, we should fix it.
>>> Regards,
>>> Andor
>>>> On 2021. Feb 9., at 13:45, Justin Ling Mao <ma...@sina.com>
>>> wrote:
>>>>
>>>> Oops, it's my blame. I'm very sorry for my mistakes. Since these days
>>> the CI is in disorder and it's a typo, so I'm not waiting for CI check and
>>> forgot that an UT has covered this change although I wrote these related
>>> codes. It's all my mistake and I will summarize our submission process and
>>> this accident. I will write another letter to discuss the commit rules and
>>> how to improve our code review throughput
>>>>
>>>>
>>>> ----- Original Message -----
>>>> From: Andor Molnar <an...@apache.org>
>>>> To: DevZooKeeper <de...@zookeeper.apache.org>
>>>> Subject: Commit 4faf507 broke the build
>>>> Date: 2021-02-09 19:43
>>>>
>>>> Hi,
>>>> I noticed that the latest commit 4faf507 ZOOKEEPER-4007: A typo in the
>>> ZKUtil#validateFileInput method broke the build, because the unit test has
>>> not been amended.
>>>> I reverted the commit to fix the build. Please create new PR with a
>>> proper patch.
>>>> Has the committer verified that the build is green before submitting it?
>>>> Andor
>>>
Re: Re: [Commit Accident Case Study] Commit 4faf507 broke the build
Posted by Benjamin Reed <br...@apache.org>.
thank you for figuring this out! two questions:
1) is there a daily test build still running? if so, where can we see
its status?
2) what is the easiest way to retrigger tests? (sorry, i know i've
asked this before :'( )
ben
On Wed, Feb 10, 2021 at 2:18 AM Szalay-Bekő Máté
<sz...@gmail.com> wrote:
>
> > For punishment:
> > I will frozen/forbid my committership permission for three months
>
> I think you took this too seriously. Mistakes / accidents happen when
> someone is working (I did much more serious ones myself on different
> projects). And the community is grateful for the contribution, no one
> should expect perfection. At least I hope so, for my sake :p
>
> Independently from this issue we really should focus on making our CI to be
> rock-solid. So if the CI is red, then we could assume the PR broke
> something. Currently I think flaky tests and independent CI issues are more
> frequently causing red builds than actual failures introduced by PRs.
>
> Cheers,
> Mate
>
> On Wed, Feb 10, 2021 at 10:26 AM Justin Ling Mao <ma...@sina.com>
> wrote:
>
> > Haha, it scared me. Let me go through this accident.
> > The root cause is: I'm over-confident, frivolous and hasty. I flatter
> > myself that it's just a typo and committing it could not have anything bad
> > happens. And I also don't give this PR a buffer time for other people's
> > review.
> > Accident is bad, but it's much more terrible if we can not reflect on it
> > and think about how to avoid it next time.
> > For remedy:
> > I will add a new section: Commit Accident Case Study in [1] for the
> > successor’s learning (Can anyone give me the permission to edit that wiki)?
> > I will sum up our commit rules and the checklists before committing one
> > patch, and do some works to use the Github CI and commit script to
> > protect/check these constraint.
> > For punishment:
> > I will frozen/forbid my committership permission for three months(02-10 ~
> > 05-10). During this period, I must not commit anything. I wish I could
> > reflect on my fault and have a better understanding on the wording: "With
> > great power comes great responsibility"
> >
> > Reference:[1]
> > https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
> > ----- Original Message -----
> > From: Andor Molnar <an...@apache.org>
> > To: maoling199210191@sina.com
> > Cc: dev <de...@zookeeper.apache.org>
> > Subject: Re: Commit 4faf507 broke the build
> > Date: 2021-02-10 00:26
> >
> > I’m sorry Justin. There’s no excuse for a mistake like this. We should not
> > show mercy for anybody, otherwise it would erode the trust in our
> > community. Your committership is now revoked.
> > Just kidding. Don’t worry at all. ;-)
> > I reverted the patch, so now please create a new PR with all the required
> > changes included.
> > Also I second Enrico’s comment: if CI is in bad shape, we should fix it.
> > Regards,
> > Andor
> > > On 2021. Feb 9., at 13:45, Justin Ling Mao <ma...@sina.com>
> > wrote:
> > >
> > > Oops, it's my blame. I'm very sorry for my mistakes. Since these days
> > the CI is in disorder and it's a typo, so I'm not waiting for CI check and
> > forgot that an UT has covered this change although I wrote these related
> > codes. It's all my mistake and I will summarize our submission process and
> > this accident. I will write another letter to discuss the commit rules and
> > how to improve our code review throughput
> > >
> > >
> > > ----- Original Message -----
> > > From: Andor Molnar <an...@apache.org>
> > > To: DevZooKeeper <de...@zookeeper.apache.org>
> > > Subject: Commit 4faf507 broke the build
> > > Date: 2021-02-09 19:43
> > >
> > > Hi,
> > > I noticed that the latest commit 4faf507 ZOOKEEPER-4007: A typo in the
> > ZKUtil#validateFileInput method broke the build, because the unit test has
> > not been amended.
> > > I reverted the commit to fix the build. Please create new PR with a
> > proper patch.
> > > Has the committer verified that the build is green before submitting it?
> > > Andor
> >
Re: Re: [Commit Accident Case Study] Commit 4faf507 broke the build
Posted by Szalay-Bekő Máté <sz...@gmail.com>.
> For punishment:
> I will frozen/forbid my committership permission for three months
I think you took this too seriously. Mistakes / accidents happen when
someone is working (I did much more serious ones myself on different
projects). And the community is grateful for the contribution, no one
should expect perfection. At least I hope so, for my sake :p
Independently from this issue we really should focus on making our CI to be
rock-solid. So if the CI is red, then we could assume the PR broke
something. Currently I think flaky tests and independent CI issues are more
frequently causing red builds than actual failures introduced by PRs.
Cheers,
Mate
On Wed, Feb 10, 2021 at 10:26 AM Justin Ling Mao <ma...@sina.com>
wrote:
> Haha, it scared me. Let me go through this accident.
> The root cause is: I'm over-confident, frivolous and hasty. I flatter
> myself that it's just a typo and committing it could not have anything bad
> happens. And I also don't give this PR a buffer time for other people's
> review.
> Accident is bad, but it's much more terrible if we can not reflect on it
> and think about how to avoid it next time.
> For remedy:
> I will add a new section: Commit Accident Case Study in [1] for the
> successor’s learning (Can anyone give me the permission to edit that wiki)?
> I will sum up our commit rules and the checklists before committing one
> patch, and do some works to use the Github CI and commit script to
> protect/check these constraint.
> For punishment:
> I will frozen/forbid my committership permission for three months(02-10 ~
> 05-10). During this period, I must not commit anything. I wish I could
> reflect on my fault and have a better understanding on the wording: "With
> great power comes great responsibility"
>
> Reference:[1]
> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
> ----- Original Message -----
> From: Andor Molnar <an...@apache.org>
> To: maoling199210191@sina.com
> Cc: dev <de...@zookeeper.apache.org>
> Subject: Re: Commit 4faf507 broke the build
> Date: 2021-02-10 00:26
>
> I’m sorry Justin. There’s no excuse for a mistake like this. We should not
> show mercy for anybody, otherwise it would erode the trust in our
> community. Your committership is now revoked.
> Just kidding. Don’t worry at all. ;-)
> I reverted the patch, so now please create a new PR with all the required
> changes included.
> Also I second Enrico’s comment: if CI is in bad shape, we should fix it.
> Regards,
> Andor
> > On 2021. Feb 9., at 13:45, Justin Ling Mao <ma...@sina.com>
> wrote:
> >
> > Oops, it's my blame. I'm very sorry for my mistakes. Since these days
> the CI is in disorder and it's a typo, so I'm not waiting for CI check and
> forgot that an UT has covered this change although I wrote these related
> codes. It's all my mistake and I will summarize our submission process and
> this accident. I will write another letter to discuss the commit rules and
> how to improve our code review throughput
> >
> >
> > ----- Original Message -----
> > From: Andor Molnar <an...@apache.org>
> > To: DevZooKeeper <de...@zookeeper.apache.org>
> > Subject: Commit 4faf507 broke the build
> > Date: 2021-02-09 19:43
> >
> > Hi,
> > I noticed that the latest commit 4faf507 ZOOKEEPER-4007: A typo in the
> ZKUtil#validateFileInput method broke the build, because the unit test has
> not been amended.
> > I reverted the commit to fix the build. Please create new PR with a
> proper patch.
> > Has the committer verified that the build is green before submitting it?
> > Andor
>