Re: [openstack-dev] [nova] stable branches failure to handle review backlog
Mark McLoughlin wrote: [...] I don't see how any self-respecting open-source project can throw a release over the wall and have no ability to address critical bugs with that release until the next release 6 months later which will also include a bunch of new feature work with new bugs. That's not a distro maintainer point of view. I agree that the job changed a bit since those early days, and now apart from a very small group of stable specialists (mostly the stable release managers), everyone else on stable-core is actually specialized in a given project. It would make sense for each project to have a set of dedicated stable liaisons that would work together with stable release managers in getting critical bugfixes to stable branch point releases. Relying on the same small group of people now that we have 10+ projects to cover is unreasonable. There are two issues to solve before we do that, though. The projects have to be OK with taking on that extra burden (it becomes their responsibility to dedicate resources to stable branches). And we need to make sure the stable branch guidelines are well communicated. [...] That's quite a leap to say that -core team members will be so incapable of the appropriate level of conservatism that the branch will be killed. The idea behind not automatically granting +2 on stable/* to -core members was simply we didn't want people diving in and approving unsuitable stuff out of ignorance. I could totally see an argument now that everyone is a lot more familiar with gerrit, the concept of stable releases is well established and we should be able to trust -core team members to learn how to make the risk vs benefit tradeoffs needed for stable reviews. The question here is whether every -core member on a project should automatically be on stable-core (and we can reuse the same group) or do we have to maintain two groups. From my experience reviewing stable branch changes, I see two types of issues with regular core doing stable reviews. There is the accidental stable/* review, where the person thinks he is reviewing a master review. Gerrit makes it look extremely similar, so more often than not, we have -core members wondering why they don't have +2 on review X, or core members doing a code review (criticizing the code again) rather than a stable review (checking the type of change and the backport). Maybe we could solve that by making gerrit visually different on stable reviews ? And then there is the opportunistic review, where a core member bends the stable rules because he wants a specific fix backported. So we get database migrations, new configuration options, or default behavior changes +1ed or +2ed in stable. When we discuss those on the review, the -core person generally disagrees with the stable rules and would like them relaxed. This is why I tend to prefer an opt-in system where the core member signs up to do stable review. He clearly says he agrees with the stable rules and will follow them. He also signs up to do enough of them to limit the opportunistic and accidental issues. -- Thierry Carrez (ttx) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 18/08/14 11:00, Thierry Carrez wrote: Mark McLoughlin wrote: [...] I don't see how any self-respecting open-source project can throw a release over the wall and have no ability to address critical bugs with that release until the next release 6 months later which will also include a bunch of new feature work with new bugs. That's not a distro maintainer point of view. I agree that the job changed a bit since those early days, and now apart from a very small group of stable specialists (mostly the stable release managers), everyone else on stable-core is actually specialized in a given project. It would make sense for each project to have a set of dedicated stable liaisons that would work together with stable release managers in getting critical bugfixes to stable branch point releases. Relying on the same small group of people now that we have 10+ projects to cover is unreasonable. Indeed, not everyone feels safe when reviewing stable backports for projects he is not really involved in. For example, I'm more or less fine with Neutron and Oslo, and some Nova patches, but once it gets to e.g. Horizon or Keystone, I feel very worried that I even have +2 for those branches, and generally don't apply it unless I'm completely sure the fix is obvious, and the bug is clear for outsider (=me). The problem is that some projects lack people that are both confident in the code base and are dedicated to support stable maint effort. So while Neutron or Nova do not feel great deficit in the number of maintainers who care about their stable branches, for other projects it may be really hard to find anyone to review the code. Dedicated liasons would solve that. There are two issues to solve before we do that, though. The projects have to be OK with taking on that extra burden (it becomes their responsibility to dedicate resources to stable branches). And we need to make sure the stable branch guidelines are well communicated. [...] That's quite a leap to say that -core team members will be so incapable of the appropriate level of conservatism that the branch will be killed. The idea behind not automatically granting +2 on stable/* to -core members was simply we didn't want people diving in and approving unsuitable stuff out of ignorance. I could totally see an argument now that everyone is a lot more familiar with gerrit, the concept of stable releases is well established and we should be able to trust -core team members to learn how to make the risk vs benefit tradeoffs needed for stable reviews. The question here is whether every -core member on a project should automatically be on stable-core (and we can reuse the same group) or do we have to maintain two groups. From my experience reviewing stable branch changes, I see two types of issues with regular core doing stable reviews. There is the accidental stable/* review, where the person thinks he is reviewing a master review. Gerrit makes it look extremely similar, so more often than not, we have -core members wondering why they don't have +2 on review X, or core members doing a code review (criticizing the code again) rather than a stable review (checking the type of change and the backport). Maybe we could solve that by making gerrit visually different on stable reviews ? I would be glad if stable patches are not shown by default in queries unless explicitly requested. That would solve the issue of people reviewing the code of backports, and others. I would also like to have some way to explicitly mark a stable branch patch as being 'not a trivial cherry-pick'. Sometimes stable branch patches are not really similar to their original master fellows. Or even we need to push a stable branch only patch because the issue or the affected code is not in master. In that case, stable maintainers are not the ones to do review, because the code should be checked by the real cores of the affected projects, and checking technical details and general applicability of the patch for stable branch is not enough. Something like a button to mark the backport as requiring special attention from cores. Both the split and a way to drive cores' attention to specific backport would remove burden from both project cores and stable maintainers that sometimes struggle to review non-trivial bug fixes. And then there is the opportunistic review, where a core member bends the stable rules because he wants a specific fix backported. So we get database migrations, new configuration options, or default behavior changes +1ed or +2ed in stable. When we discuss those on the review, the -core person generally disagrees with the stable rules and would like them relaxed. This is why I tend to prefer an opt-in system where the core member signs up to do stable review. He clearly says he agrees with the stable rules and will follow them. He also signs up to do enough of them to
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On Tue, 2014-07-29 at 14:04 +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : On 29/07/14 12:15, Daniel P. Berrange wrote: Looking at the current review backlog I think that we have to seriously question whether our stable branch review process in Nova is working to an acceptable level On Havana - 43 patches pending - 19 patches with a single +2 - 1 patch with a -1 - 0 patches wit a -2 - Stalest waiting 111 days since most recent patch upload - Oldest waiting 250 days since first patch upload - 26 patches waiting more than 1 month since most recent upload - 40 patches waiting more than 1 month since first upload On Icehouse: - 45 patches pending - 17 patches with a single +2 - 4 patches with a -1 - 1 patch with a -2 - Stalest waiting 84 days since most recent patch upload - Oldest waiting 88 days since first patch upload - 10 patches waiting more than 1 month since most recent upload - 29 patches waiting more than 1 month since first upload I think those stats paint a pretty poor picture of our stable branch review process, particularly Havana. It should not take us 250 days for our review team to figure out whether a patch is suitable material for a stable branch, nor should we have nearly all the patches waiting more than 1 month in Havana. These branches are not getting sufficient reviewer attention and we need to take steps to fix that. If I had to set a benchmark, assuming CI passes, I'd expect us to either approve or reject submissions for stable within a 2 week window in the common case, 1 month at the worst case. Totally agreed. A bit of history. At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. I'm not sure how much backporting was going on at the time of the Essex summit. I'm sure Ubuntu had some backports, but that was probably about it? At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. During that first design summit session, it was mainly you, me and Daviey discussing. Both you and Daviey saw this primarily about distros collaborating, but I never saw it that way. I don't see how any self-respecting open-source project can throw a release over the wall and have no ability to address critical bugs with that release until the next release 6 months later which will also include a bunch of new feature work with new bugs. That's not a distro maintainer point of view. At that Essex summit, we were lamenting how many critical bugs in Nova had been discovered shortly after the Diablo release. Our inability to do a bugfix release of Nova for Diablo seemed like a huge problem to me. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. You can certainly argue that the project never signed up for the responsibility. I don't see it that way, but there was certainly always a debate whether this was the project taking responsibility for bugfix releases or whether it was just downstream distros collaborating. The thing about branches going away if they're not maintained isn't anything unusual. If *any* effort within the project becomes so unmaintained due to a lack of interest such that we can't stand over it, then we should consider retiring it. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. That's quite a leap to say that -core team members will be so incapable of the appropriate level of conservatism that the branch will be
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
Russell Bryant wrote: On 07/30/2014 01:22 PM, Kevin L. Mitchell wrote: Maybe what we need to do is give *-core permission to +2 the patches, but only stable/maint team has *approval* permission. Then, the cores can review the code, and stable/maint only has to verify applicability to the stable branch… That's a good idea. We would probably switch to $PROJECT-stable-maint teams then (each including $PROJECT-core and the general stable-maint team) since we don't have a group in Gerrit for *-core anyway. We could also just do this by convention. We already allow a single +2 on backports if the person who did the backport is also on stable-maint. We could add to that allowing a single +2/+A if the person who did the backport is on project-core, or if a person from project-core has given a +1. That would avoid the above reconfiguration, but then the stable-maint people have to check which +1s are actually $PROJECT-core +1s... which sounds like a painful task. Cheers, -- Thierry Carrez (ttx) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 30/07/14 19:22, Kevin L. Mitchell wrote: On Wed, 2014-07-30 at 09:01 +0200, Flavio Percoco wrote: As a stable-maint, I'm always hesitant to review patches I've no understanding on, hence I end up just checking how big is the patch, whether it adds/removes new configuration options etc but, the real review has to be done by someone with good understanding of the change. Something I've done in the past is adding the folks that had approved the patch on master to the stable/maint review. They should know that code already, which means it shouldn't take them long to review it. All the sanity checks should've been done already. With all that said, I'd be happy to give *-core approval permissions on stable branches, but I still think we need a dedicated team that has a final (or at least relevant) word on the patches. Maybe what we need to do is give *-core permission to +2 the patches, but only stable/maint team has *approval* permission. Then, the cores can review the code, and stable/maint only has to verify applicability to the stable branch… +2 for the idea. :) Cheers, /Ihar -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJT2hg4AAoJEC5aWaUY1u57xJUH+waJ3GKorN/8gxr6XjY330+L 60UmdVZ/WHZcj3GPm7ZjNqMQCdoXRWnmZLBUry+GzpuHcMXSHRW/H/bMiAVhufP4 +VWXu6SlIfzlSRWZNkI4k2uoyfYIJKcpPZJTpU51fjC0JWKmdwHUBB/YBa41oC9f BV3xVih389sbfLPUO8nvbjHk6Vo+5eCbzqXhefVxOnuP+mbaIJTek6G5lmxU/Vem LdUdVwN2pCC4nt6JF+WDme6DZ6CV8lwbu1cgqNJojZFp+F1vFYIU0oyQrBY11/96 7NHrsVecI2MvGtPsl3S+T9INV4Z35BAsfJ5Qac/KzGmLCL/aapkQgv8KxPPqI4w= =+oPU -END PGP SIGNATURE- ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 2014-07-31 10:17:16 +0200 (+0200), Thierry Carrez wrote: That's a good idea. We would probably switch to $PROJECT-stable-maint teams then (each including $PROJECT-core and the general stable-maint team) since we don't have a group in Gerrit for *-core anyway. [...] I think we can actually do this with the groups we already have, just by changing the exclusive group permissions to only be for workflow -1..+1 rather than also code review -2..+2? That way the stable branches start inheriting the code review permissions from the parent refs/* blocks in per-project ACLs. We could try it out after the current point release freeze wraps up, if there's some consensus. -- Jeremy Stanley ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
Jeremy Stanley wrote: On 2014-07-31 10:17:16 +0200 (+0200), Thierry Carrez wrote: That's a good idea. We would probably switch to $PROJECT-stable-maint teams then (each including $PROJECT-core and the general stable-maint team) since we don't have a group in Gerrit for *-core anyway. [...] I think we can actually do this with the groups we already have, just by changing the exclusive group permissions to only be for workflow -1..+1 rather than also code review -2..+2? That way the stable branches start inheriting the code review permissions from the parent refs/* blocks in per-project ACLs. We could try it out after the current point release freeze wraps up, if there's some consensus. Sounds promising. If I understand correctly, that would give core people +2 rights on their project stable/*, but keep APRV rights to stable-maint... That said, after having spent the day on stable reviews, I confirm that in most cases reviewers that are not specifically on stable-maint tend to review the correctness of the patch (like for a master branch review) rather than only review its applicability to the stable branch (which is what matters here). So it looks like we would have to train a lot of people to the delicate art of stable branch review before those +2 would actually help us in the review. -- Thierry Carrez (ttx) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 07/29/2014 09:01 PM, Russell Bryant wrote: On 07/29/2014 12:12 PM, Daniel P. Berrange wrote: Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. I agree with this. If we can't trust someone on *-core to follow the stable criteria, then they shouldn't be on *-core in the first place. Further, if we can't trust the combination of *two* people from *-core to approve a stable backport, then we're really in trouble. +1 As a stable-maint, I'm always hesitant to review patches I've no understanding on, hence I end up just checking how big is the patch, whether it adds/removes new configuration options etc but, the real review has to be done by someone with good understanding of the change. Something I've done in the past is adding the folks that had approved the patch on master to the stable/maint review. They should know that code already, which means it shouldn't take them long to review it. All the sanity checks should've been done already. With all that said, I'd be happy to give *-core approval permissions on stable branches, but I still think we need a dedicated team that has a final (or at least relevant) word on the patches. Flavio -- @flaper87 Flavio Percoco ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 29/07/14 18:12, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 08:30:09AM -0700, Jay Pipes wrote: On 07/29/2014 06:13 AM, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 02:04:42PM +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. Adding more people to the stable team won't kill those branches. I'm not suggesting we change the criteria for accepting patches, or that we dramatically increase the number of patches we accept. There is clearly alot of stuff proposed to stable that the existing stable team think is a good idea - as illustrated by the number of patches with at least one +2 present. On the contrary, having a bigger stable team comprises all of core + interested distro maintainers will ensure that the stable branches are actually gettting the patches people in the field need to provide a stable cloud. -1 In my experience, the distro maintainers who pioneered the stable branch teams had opposite viewpoints to core teams in regards to what was appropriate to put into a stable release. I think it's dangerous to populate the stable team with the core team members just because of long review and merge times. Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. Still, it's quite common to see patches with no cherry-pick info, or Conflicts section removed, or incorrect Change-Id being approved and pushed. It's also common for people to review backports as if they are sent against master (with all nit picking, unit test changes requested, no consideration for stable branch applicability...) Regards, Daniel -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJT2LfQAAoJEC5aWaUY1u570woIAME9gwi7MlgqhXnxMwbnn+fv Exrh3zSXfCvxQ76F5z0CVxu04bItdI4ZfYOU+/Emxx+ay0evGhHPP5jjvUYhsbED h6Nqf/yefLQZkhuwKTNyiV3FObOsPG+Cu6yAYn8y/eUyGypQfx5oXLSof+GkzMhf bMtVdxJawfWyrUJjuQpaAbZbpGzCDjqoBsZK+28RMxraBJDsz2qKpzrUp5xH71Wb NuyKSzM5/wsbW6NtUeRESJ7HtvT8kcl1CbYAkTvbjpY4+zN2Xe63Tf62cKC30eli XPP3feaPPaudUznfRLhVqRB1Yel/f0+wyj7p9NnO6cASOx2OO1O4GCI3+E2ElnY= =hP7m -END PGP SIGNATURE- ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On Wed, Jul 30, 2014 at 11:16:00AM +0200, Ihar Hrachyshka wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 29/07/14 18:12, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 08:30:09AM -0700, Jay Pipes wrote: On 07/29/2014 06:13 AM, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 02:04:42PM +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. Adding more people to the stable team won't kill those branches. I'm not suggesting we change the criteria for accepting patches, or that we dramatically increase the number of patches we accept. There is clearly alot of stuff proposed to stable that the existing stable team think is a good idea - as illustrated by the number of patches with at least one +2 present. On the contrary, having a bigger stable team comprises all of core + interested distro maintainers will ensure that the stable branches are actually gettting the patches people in the field need to provide a stable cloud. -1 In my experience, the distro maintainers who pioneered the stable branch teams had opposite viewpoints to core teams in regards to what was appropriate to put into a stable release. I think it's dangerous to populate the stable team with the core team members just because of long review and merge times. Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. Still, it's quite common to see patches with no cherry-pick info, or Conflicts section removed, or incorrect Change-Id being approved and pushed. It's also common for people to review backports as if they are sent against master (with all nit picking, unit test changes requested, no consideration for stable branch applicability...) As said before, if people reviewing the code consistently fail to follow the agreed review guidelines, then point out what they're doing wrong and if they fail to improve, remove their +2 privileges. There's nothing unique about stable branches in this regard. If people reviewing master consistently did the wrong thing they'd have their +2 removed too. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
Russell Bryant wrote: On 07/29/2014 12:12 PM, Daniel P. Berrange wrote: Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. I agree with this. If we can't trust someone on *-core to follow the stable criteria, then they shouldn't be on *-core in the first place. Further, if we can't trust the combination of *two* people from *-core to approve a stable backport, then we're really in trouble. There are a few different facets on this issue. The first facet is a community aspect. Stable branch maintenance is a task separate from upstream development, ideally performed by the people that have a direct interest in having good, maintained stable branches (downstream distribution packagers). Now, if *all PTLs* are fine with adding stable branch maintenance to the tasks of their core reviewers (in addition to specs and master branch review), then I guess we can abandon that concept of separate tasks. But I wasn't under the impression the core population was looking forward to add extra duties to their current workload. The second facet is the opt-in nature of the job. Yes, core reviewers are trusted with acceptation of patches on the master branch and probably have the capacity to evaluate stable branch patches as well. But stable branches have different rules, and most current core reviewers don't know them. There have been numerous cases where an inappropriate patch was pushed to stable/* by a core developer, who was very insistent on having his bugfix merged and rejected the rules we were opposing to them. I'm fine with adding core reviewers which have read and agreed to follow the stable rules, but adding them all by default sounds more like a convenient way to push your own patches in than a way to solve the stable manpower issue. The third facet is procedural: we currently have a single stable-maint team for all integrated projects. The original idea is that the stable branch maintainers do not judge the patch itself (which was judged by -core people when the patch landed in master), they just judge if it's appropriate for stable backport (how disruptive it is, if it's feature-y or if it adds a new config option, or if it changes default behavior). You don't need that much to be a domain expert to judge that, and if unsure we would just ask. If we add more project-specific core reviewers, we should probably switch to per-project stable-maint groups. I'm not opposed to that, just saying that's an infra config change we need to push. I'm generally open to changing that, since I reckon we have a manpower issue on the stable maint side. I just want to make sure everyone knows what they are signing up for here. We would do it for all the projects, we can't special-case Nova. -- Thierry Carrez (ttx) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 07/30/2014 02:21 AM, Daniel P. Berrange wrote: On Wed, Jul 30, 2014 at 11:16:00AM +0200, Ihar Hrachyshka wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 29/07/14 18:12, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 08:30:09AM -0700, Jay Pipes wrote: On 07/29/2014 06:13 AM, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 02:04:42PM +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. Adding more people to the stable team won't kill those branches. I'm not suggesting we change the criteria for accepting patches, or that we dramatically increase the number of patches we accept. There is clearly alot of stuff proposed to stable that the existing stable team think is a good idea - as illustrated by the number of patches with at least one +2 present. On the contrary, having a bigger stable team comprises all of core + interested distro maintainers will ensure that the stable branches are actually gettting the patches people in the field need to provide a stable cloud. -1 In my experience, the distro maintainers who pioneered the stable branch teams had opposite viewpoints to core teams in regards to what was appropriate to put into a stable release. I think it's dangerous to populate the stable team with the core team members just because of long review and merge times. Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. Still, it's quite common to see patches with no cherry-pick info, or Conflicts section removed, or incorrect Change-Id being approved and pushed. It's also common for people to review backports as if they are sent against master (with all nit picking, unit test changes requested, no consideration for stable branch applicability...) As said before, if people reviewing the code consistently fail to follow the agreed review guidelines, then point out what they're doing wrong and if they fail to improve, remove their +2 privileges. There's nothing unique about stable branches in this regard. If people reviewing master consistently did the wrong thing they'd have their +2 removed too. Honestly, I think if anyone wants to be added to stable-maint, they should raise their hand and join the team. I'm sure they would be welcomed. I don't feel that stable maintenance is what we actually asked the core teams to do as a community. The risks are much higher for people approving stable backports, so it should be something people explicitly get in the mindset to do. I think it's also
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On Wed, Jul 30, 2014 at 7:52 AM, Thierry Carrez thie...@openstack.org wrote: Russell Bryant wrote: On 07/29/2014 12:12 PM, Daniel P. Berrange wrote: Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. I agree with this. If we can't trust someone on *-core to follow the stable criteria, then they shouldn't be on *-core in the first place. Further, if we can't trust the combination of *two* people from *-core to approve a stable backport, then we're really in trouble. There are a few different facets on this issue. The first facet is a community aspect. Stable branch maintenance is a task separate from upstream development, ideally performed by the people that have a direct interest in having good, maintained stable branches (downstream distribution packagers). Now, if *all PTLs* are fine with adding stable branch maintenance to the tasks of their core reviewers (in addition to specs and master branch review), then I guess we can abandon that concept of separate tasks. But I wasn't under the impression the core population was looking forward to add extra duties to their current workload. Speaking as a PTL, I don't think adding all core team members for projects as stable maintainers would be a good thing. As pointed out already in this thread, the task of code reviews for stable is much different than that of code review for master. Having a more focused group of stable reviewers is likely a better solution than adding a large group of reviewers focused on upstream work. Also, as Sean pointed out in a followup email here, we have a process for people to propose themselves to stable maintenance [1]. Perhaps we need to publicize that a bit more to get more people who can actively help in the stable reviews to join the team. Thanks, Kyle [1] https://wiki.openstack.org/wiki/StableBranch#Joining_the_Team The second facet is the opt-in nature of the job. Yes, core reviewers are trusted with acceptation of patches on the master branch and probably have the capacity to evaluate stable branch patches as well. But stable branches have different rules, and most current core reviewers don't know them. There have been numerous cases where an inappropriate patch was pushed to stable/* by a core developer, who was very insistent on having his bugfix merged and rejected the rules we were opposing to them. I'm fine with adding core reviewers which have read and agreed to follow the stable rules, but adding them all by default sounds more like a convenient way to push your own patches in than a way to solve the stable manpower issue. The third facet is procedural: we currently have a single stable-maint team for all integrated projects. The original idea is that the stable branch maintainers do not judge the patch itself (which was judged by -core people when the patch landed in master), they just judge if it's appropriate for stable backport (how disruptive it is, if it's feature-y or if it adds a new config option, or if it changes default behavior). You don't need that much to be a domain expert to judge that, and if unsure we would just ask. If we add more project-specific core reviewers, we should probably switch to per-project stable-maint groups. I'm not opposed to that, just saying that's an infra config change we need to push. I'm generally open to changing that, since I reckon we have a manpower issue on the stable maint side. I just want to make sure everyone knows what they are signing up for here. We would do it for all the projects, we can't special-case Nova. -- Thierry Carrez (ttx) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On Wed, 2014-07-30 at 09:01 +0200, Flavio Percoco wrote: As a stable-maint, I'm always hesitant to review patches I've no understanding on, hence I end up just checking how big is the patch, whether it adds/removes new configuration options etc but, the real review has to be done by someone with good understanding of the change. Something I've done in the past is adding the folks that had approved the patch on master to the stable/maint review. They should know that code already, which means it shouldn't take them long to review it. All the sanity checks should've been done already. With all that said, I'd be happy to give *-core approval permissions on stable branches, but I still think we need a dedicated team that has a final (or at least relevant) word on the patches. Maybe what we need to do is give *-core permission to +2 the patches, but only stable/maint team has *approval* permission. Then, the cores can review the code, and stable/maint only has to verify applicability to the stable branch… -- Kevin L. Mitchell kevin.mitch...@rackspace.com Rackspace ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 7/30/14, 8:22 PM, Kevin L. Mitchell kevin.mitch...@rackspace.com wrote: On Wed, 2014-07-30 at 09:01 +0200, Flavio Percoco wrote: As a stable-maint, I'm always hesitant to review patches I've no understanding on, hence I end up just checking how big is the patch, whether it adds/removes new configuration options etc but, the real review has to be done by someone with good understanding of the change. Something I've done in the past is adding the folks that had approved the patch on master to the stable/maint review. They should know that code already, which means it shouldn't take them long to review it. All the sanity checks should've been done already. With all that said, I'd be happy to give *-core approval permissions on stable branches, but I still think we need a dedicated team that has a final (or at least relevant) word on the patches. Maybe what we need to do is give *-core permission to +2 the patches, but only stable/maint team has *approval* permission. Then, the cores can review the code, and stable/maint only has to verify applicability to the stable branchŠ +1 -- Kevin L. Mitchell kevin.mitch...@rackspace.com Rackspace ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 07/30/2014 01:22 PM, Kevin L. Mitchell wrote: On Wed, 2014-07-30 at 09:01 +0200, Flavio Percoco wrote: As a stable-maint, I'm always hesitant to review patches I've no understanding on, hence I end up just checking how big is the patch, whether it adds/removes new configuration options etc but, the real review has to be done by someone with good understanding of the change. Something I've done in the past is adding the folks that had approved the patch on master to the stable/maint review. They should know that code already, which means it shouldn't take them long to review it. All the sanity checks should've been done already. With all that said, I'd be happy to give *-core approval permissions on stable branches, but I still think we need a dedicated team that has a final (or at least relevant) word on the patches. Maybe what we need to do is give *-core permission to +2 the patches, but only stable/maint team has *approval* permission. Then, the cores can review the code, and stable/maint only has to verify applicability to the stable branch… We could also just do this by convention. We already allow a single +2 on backports if the person who did the backport is also on stable-maint. We could add to that allowing a single +2/+A if the person who did the backport is on project-core, or if a person from project-core has given a +1. -- Russell Bryant ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
From: Gary Kotton [mailto:gkot...@vmware.com] On 7/30/14, 8:22 PM, Kevin L. Mitchell kevin.mitch...@rackspace.com wrote: On Wed, 2014-07-30 at 09:01 +0200, Flavio Percoco wrote: As a stable-maint, I'm always hesitant to review patches I've no understanding on, hence I end up just checking how big is the patch, whether it adds/removes new configuration options etc but, the real review has to be done by someone with good understanding of the change. Something I've done in the past is adding the folks that had approved the patch on master to the stable/maint review. They should know that code already, which means it shouldn't take them long to review it. All the sanity checks should've been done already. With all that said, I'd be happy to give *-core approval permissions on stable branches, but I still think we need a dedicated team that has a final (or at least relevant) word on the patches. Maybe what we need to do is give *-core permission to +2 the patches, but only stable/maint team has *approval* permission. Then, the cores can review the code, and stable/maint only has to verify applicability to the stable branchŠ +1 +1 This approach guarantees final say by the stable/maint team, but lets any core validate that the patch is appropriate from the project's technical perspective. It keeps the balance but broadens the validation pool. --Rocky -- Kevin L. Mitchell kevin.mitch...@rackspace.com Rackspace ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [nova] stable branches failure to handle review backlog
Looking at the current review backlog I think that we have to seriously question whether our stable branch review process in Nova is working to an acceptable level On Havana - 43 patches pending - 19 patches with a single +2 - 1 patch with a -1 - 0 patches wit a -2 - Stalest waiting 111 days since most recent patch upload - Oldest waiting 250 days since first patch upload - 26 patches waiting more than 1 month since most recent upload - 40 patches waiting more than 1 month since first upload On Icehouse: - 45 patches pending - 17 patches with a single +2 - 4 patches with a -1 - 1 patch with a -2 - Stalest waiting 84 days since most recent patch upload - Oldest waiting 88 days since first patch upload - 10 patches waiting more than 1 month since most recent upload - 29 patches waiting more than 1 month since first upload I think those stats paint a pretty poor picture of our stable branch review process, particularly Havana. It should not take us 250 days for our review team to figure out whether a patch is suitable material for a stable branch, nor should we have nearly all the patches waiting more than 1 month in Havana. These branches are not getting sufficient reviewer attention and we need to take steps to fix that. If I had to set a benchmark, assuming CI passes, I'd expect us to either approve or reject submissions for stable within a 2 week window in the common case, 1 month at the worst case. If we are trying to throttle down the rate of change in Havana, that totally makes sense, but we should be more active at rejecting patches if that is our current goal, not let them hang around in limbo for many months. I'm actually unclear on who even has permission to approve patches on stable branches ? Despite being in Nova core I don't have any perm to approve patches on stable. I think it is pretty odd that we've got a system where the supposed experts of the Nova team can't approve patches for stable. I get that we've probably got people on stable team who are not in core, but IMHO we should have the stable team comprising a superset of core, not a subset. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 29/07/14 12:15, Daniel P. Berrange wrote: Looking at the current review backlog I think that we have to seriously question whether our stable branch review process in Nova is working to an acceptable level On Havana - 43 patches pending - 19 patches with a single +2 - 1 patch with a -1 - 0 patches wit a -2 - Stalest waiting 111 days since most recent patch upload - Oldest waiting 250 days since first patch upload - 26 patches waiting more than 1 month since most recent upload - 40 patches waiting more than 1 month since first upload On Icehouse: - 45 patches pending - 17 patches with a single +2 - 4 patches with a -1 - 1 patch with a -2 - Stalest waiting 84 days since most recent patch upload - Oldest waiting 88 days since first patch upload - 10 patches waiting more than 1 month since most recent upload - 29 patches waiting more than 1 month since first upload I think those stats paint a pretty poor picture of our stable branch review process, particularly Havana. It should not take us 250 days for our review team to figure out whether a patch is suitable material for a stable branch, nor should we have nearly all the patches waiting more than 1 month in Havana. These branches are not getting sufficient reviewer attention and we need to take steps to fix that. If I had to set a benchmark, assuming CI passes, I'd expect us to either approve or reject submissions for stable within a 2 week window in the common case, 1 month at the worst case. Totally agreed. If we are trying to throttle down the rate of change in Havana, that totally makes sense, but we should be more active at rejecting patches if that is our current goal, not let them hang around in limbo for many months. Tip: to be notified in time about new backport requests, you may add those branches you're interested in to watched, in Gerrit, go to Settings - Watched Projects, and add whatever you like. Then you'll receive emails for each backport request. I'm actually unclear on who even has permission to approve patches on stable branches ? Despite being in Nova core I don't have any perm to approve patches on stable. I think it is pretty odd that we've got a system where the supposed experts of the Nova team can't approve patches for stable. I get that we've probably got people on stable team who are not in core, but IMHO we should have the stable team comprising a superset of core, not a subset. AFAIK stable team consists of project PTLs + people interested in stable branches specifically (that got added to the team after their request). Anyone can start reviewing the patches and ask to be added to the team. I also think it's weird that project cores don't have +2 for stable branches of their projects. They do not require global +2 for all stable branches though. Regards, Daniel -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJT13h/AAoJEC5aWaUY1u57Kn4H+gMhIA2omnwIfFqibrMRnTex DkZNtgDNvfPIBxkhmkj0anREsnglgrwjufPZYF0MmJcxSvCDLJnWoDJ+iOxir9sg FiW0GVcSB89TNjKNbRfeFcuP6J6Dw6eNRvYnwf2OoypcyVBN+yElHJG+8/bzZ7FV lZFGdTK3X777ik2DtFdjkpbrGxxOG+BC/ZWtiKWiI5HPnnl0ZZPHuI44cclDCvGu bcR5yjFkMYa/hXnzbM+vYcP/kf7iBguEdfn792egrZE1BajSknbT6HYXkJ4C765R qmq487hlJ60KdkSS8oEWzLcRrNIKir3qyMTqjZ73tUIuKdATcqiylC53a0ZuJKM= =B7hS -END PGP SIGNATURE- ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
Ihar Hrachyshka a écrit : On 29/07/14 12:15, Daniel P. Berrange wrote: Looking at the current review backlog I think that we have to seriously question whether our stable branch review process in Nova is working to an acceptable level On Havana - 43 patches pending - 19 patches with a single +2 - 1 patch with a -1 - 0 patches wit a -2 - Stalest waiting 111 days since most recent patch upload - Oldest waiting 250 days since first patch upload - 26 patches waiting more than 1 month since most recent upload - 40 patches waiting more than 1 month since first upload On Icehouse: - 45 patches pending - 17 patches with a single +2 - 4 patches with a -1 - 1 patch with a -2 - Stalest waiting 84 days since most recent patch upload - Oldest waiting 88 days since first patch upload - 10 patches waiting more than 1 month since most recent upload - 29 patches waiting more than 1 month since first upload I think those stats paint a pretty poor picture of our stable branch review process, particularly Havana. It should not take us 250 days for our review team to figure out whether a patch is suitable material for a stable branch, nor should we have nearly all the patches waiting more than 1 month in Havana. These branches are not getting sufficient reviewer attention and we need to take steps to fix that. If I had to set a benchmark, assuming CI passes, I'd expect us to either approve or reject submissions for stable within a 2 week window in the common case, 1 month at the worst case. Totally agreed. A bit of history. At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. If we are trying to throttle down the rate of change in Havana, that totally makes sense, but we should be more active at rejecting patches if that is our current goal, not let them hang around in limbo for many months. Tip: to be notified in time about new backport requests, you may add those branches you're interested in to watched, in Gerrit, go to Settings - Watched Projects, and add whatever you like. Then you'll receive emails for each backport request. I'm actually unclear on who even has permission to approve patches on stable branches ? Despite being in Nova core I don't have any perm to approve patches on stable. I think it is pretty odd that we've got a system where the supposed experts of the Nova team can't approve patches for stable. I get that we've probably got people on stable team who are not in core, but IMHO we should have the stable team comprising a superset of core, not a subset. AFAIK stable team consists of project PTLs + people interested in stable branches specifically (that got added to the team after their request). Anyone can start reviewing the patches and ask to be added to the team. I also think it's weird that project cores don't have +2 for stable branches of their projects. They do not require global +2 for all stable branches though. The key reason why $PROJECT-core don't automatically get stable branch +2 is that the rules for accepting a patch there are VERY different from the rules for accepting a patch for master, and most -core people don't know those. We need to ensure those -core people know the stable branch acceptance rules before we
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On Tue, Jul 29, 2014 at 02:04:42PM +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. Adding more people to the stable team won't kill those branches. I'm not suggesting we change the criteria for accepting patches, or that we dramatically increase the number of patches we accept. There is clearly alot of stuff proposed to stable that the existing stable team think is a good idea - as illustrated by the number of patches with at least one +2 present. On the contrary, having a bigger stable team comprises all of core + interested distro maintainers will ensure that the stable branches are actually gettting the patches people in the field need to provide a stable cloud. If we are trying to throttle down the rate of change in Havana, that totally makes sense, but we should be more active at rejecting patches if that is our current goal, not let them hang around in limbo for many months. Tip: to be notified in time about new backport requests, you may add those branches you're interested in to watched, in Gerrit, go to Settings - Watched Projects, and add whatever you like. Then you'll receive emails for each backport request. I'm actually unclear on who even has permission to approve patches on stable branches ? Despite being in Nova core I don't have any perm to approve patches on stable. I think it is pretty odd that we've got a system where the supposed experts of the Nova team can't approve patches for stable. I get that we've probably got people on stable team who are not in core, but IMHO we should have the stable team comprising a superset of core, not a subset. AFAIK stable team consists of project PTLs + people interested in stable branches specifically (that got added to the team after their request). Anyone can start reviewing the patches and ask to be added to the team. I also think it's weird that project cores don't have +2 for stable branches of their projects. They do not require global +2 for all stable branches though. The key reason why $PROJECT-core don't automatically get stable branch +2 is that the rules for accepting a patch there are VERY different from the rules for accepting a patch for master, and most -core people don't know those. We need to ensure those -core people know the stable branch acceptance rules before we grant them +2 there. I think that's a really weak argument against having core team to take part in the stable branch approval. If the rules are outlined somewhere on the wiki anyone I know on the core team is more than capable of reading following them. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o-
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
Daniel P. Berrange wrote: The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. I would be interested to know who is actually complaining... This was supposed to be a closed loop area: consumers of the branches are the ones who maintain it. So they can't complain it's not up to date -- it's their work to maintain it. I think that's a very self-healing situation, and changing from that (by making it yet another core reviewers duty) sounds very dangerous to me. FWIW changes tend to get picked up by the stable point release manager as they get closer to a point release. There is one due soon so I expect a peak of activity soon. Also note that stable branch maintainers have their own mailing-list: http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-stable-maint We need to ensure those -core people know the stable branch acceptance rules before we grant them +2 there. I think that's a really weak argument against having core team to take part in the stable branch approval. If the rules are outlined somewhere on the wiki anyone I know on the core team is more than capable of reading following them. They are certainly capable (and generally make good stable-maint candidates). But that's not work they signed up for. I prefer that work to be opt-in rather than add to the plate of core reviewers by making them all collectively responsible for stable branch maintenance too. Rules are here, btw: https://wiki.openstack.org/wiki/StableBranch -- Thierry Carrez (ttx) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 07/29/2014 06:13 AM, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 02:04:42PM +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. Adding more people to the stable team won't kill those branches. I'm not suggesting we change the criteria for accepting patches, or that we dramatically increase the number of patches we accept. There is clearly alot of stuff proposed to stable that the existing stable team think is a good idea - as illustrated by the number of patches with at least one +2 present. On the contrary, having a bigger stable team comprises all of core + interested distro maintainers will ensure that the stable branches are actually gettting the patches people in the field need to provide a stable cloud. -1 In my experience, the distro maintainers who pioneered the stable branch teams had opposite viewpoints to core teams in regards to what was appropriate to put into a stable release. I think it's dangerous to populate the stable team with the core team members just because of long review and merge times. Distros can and should have more people participating in the stable teams -- as should non-distro folks that deploy and care about non-master deployments. If core team members are getting pinged about certain reviews on stable branches, they should direct the pinger to the stable team members. Just my 2 cents, -jay ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On Tue, Jul 29, 2014 at 08:30:09AM -0700, Jay Pipes wrote: On 07/29/2014 06:13 AM, Daniel P. Berrange wrote: On Tue, Jul 29, 2014 at 02:04:42PM +0200, Thierry Carrez wrote: Ihar Hrachyshka a écrit : At the dawn of time there were no OpenStack stable branches, each distribution was maintaining its own stable branches, duplicating the backporting work. At some point it was suggested (mostly by RedHat and Canonical folks) that there should be collaboration around that task, and the OpenStack project decided to set up official stable branches where all distributions could share the backporting work. The stable team group was seeded with package maintainers from all over the distro world. So these branches originally only exist as a convenient place to collaborate on backporting work. This is completely separate from development work, even if those days backports are often proposed by developers themselves. The stable branch team is separate from the rest of OpenStack teams. We have always been very clear tht if the stable branches are no longer maintained (i.e. if the distributions don't see the value of those anymore), then we'll consider removing them. We, as a project, only signed up to support those as long as the distros wanted them. We have been adding new members to the stable branch teams recently, but those tend to come from development teams rather than downstream distributions, and that starts to bend the original landscape. Basically, the stable branch needs to be very conservative to be a source of safe updates -- downstream distributions understand the need to weigh the benefit of the patch vs. the disruption it may cause. Developers have another type of incentive, which is to get the fix they worked on into stable releases, without necessarily being very conservative. Adding more -core people to the stable team to compensate the absence of distro maintainers will ultimately kill those branches. The situation I'm seeing is that the broader community believe that the Nova core team is responsible for the nova stable branches. When stuff sits in review for ages it is the core team that is getting pinged about it and on the receiving end of the complaints the inaction of review. Adding more people to the stable team won't kill those branches. I'm not suggesting we change the criteria for accepting patches, or that we dramatically increase the number of patches we accept. There is clearly alot of stuff proposed to stable that the existing stable team think is a good idea - as illustrated by the number of patches with at least one +2 present. On the contrary, having a bigger stable team comprises all of core + interested distro maintainers will ensure that the stable branches are actually gettting the patches people in the field need to provide a stable cloud. -1 In my experience, the distro maintainers who pioneered the stable branch teams had opposite viewpoints to core teams in regards to what was appropriate to put into a stable release. I think it's dangerous to populate the stable team with the core team members just because of long review and merge times. Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] stable branches failure to handle review backlog
On 07/29/2014 12:12 PM, Daniel P. Berrange wrote: Sure there was some debate about what criteria were desired acceptance when stable trees were started. Once the criteria are defined I don't think it is credible to say that people are incapable of following the rules. In the unlikely event that people were to willfully ignore the agreed upon rules for stable tree, then I'd not trust them to be part of a core team working on any branch at all. With responsibility comes trust and an acceptance to follow the agreed upon processes. I agree with this. If we can't trust someone on *-core to follow the stable criteria, then they shouldn't be on *-core in the first place. Further, if we can't trust the combination of *two* people from *-core to approve a stable backport, then we're really in trouble. -- Russell Bryant ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
