On 09/30/2015 12:11 PM, Mike Perez wrote:
On 13:29 Sep 28, Ben Swartzlander wrote:
I've always thought it was a bit strange to require new drivers to
merge by milestone 1. I think I understand the motivations of the
policy. The main motivation was to free up reviewers to review "other
things" and this policy guarantees that for 75% of the release
reviewers don't have to review new drivers. The other motivation was
to prevent vendors from turning up at the last minute with crappy
drivers that needed a ton of work, by encouraging them to get started
earlier, or forcing them to wait until the next cycle.
I believe that the deadline actually does more harm than good.
First of all, to those that don't want to spend time on driver
reviews, there are other solutions to that problem. Some people do
want to review the drivers, and those who don't can simply ignore
them and spend time on what they care about. I've heard people who
spend time on driver reviews say that the milestone-1 deadline
doesn't mean they spend less time reviewing drivers overall, it just
all gets crammed into the beginning of each release. It should be
obvious that setting a deadline doesn't actually affect the amount of
reviewer effort, it just concentrates that effort.
Some bad assumptions here:
* Nobody said they didn't want to review drivers.
* "Crammed" is completely an incorrect word here. An example with last release,
we only had 3/17 drivers trying to get in during the last week of the
milestone [1]. I don't think you're very active in Cinder to really judge how
well the team has worked together to get these drivers in a timely way with
vendors.
There are fair points. No argument. I think I managed to obscure my main
point with too much assumptions and rhetoric though.
Let me restate my argument as simply as possible.
Drivers are relatively low risk to the project. They're a lot of work to
review due to the size, but the risk of missing bugs is small because
those bugs will affect only the users who choose to deploy the given
driver. Also drivers are well understood, so the process of reviewing
them is straightforward.
New features are high risk. Even a small change to the manager or API
code can have dramatic impact on all users of Cinder. Larger changes
that touch multiple modules in different areas must be reviewed by
people who understand all of Cinder just to get basic assurance that
they do what they say. Finding bugs in these kinds of changes is tricky.
Reading the code only gets you so far, and automated testing only
scratches the surface. You have to run the code and try it out. These
things take time and core team time is a limited and precious resource.
Now, if you have some high risk changes and some low risk changes, which
do you think it makes sense to work on early in the release, and which
do you think is safe to merge at the last minute? I asked myself that
question and decided that I'd rather to high risk stuff early and low
risk stuff later. Based on that belief, I'm making a suggestion to move
the deadlines around.
The argument about crappy code is also a lot weaker now that there
are CI requirements which force vendors to spend much more time up
front and clear a much higher quality bar before the driver is even
considered for merging. Drivers that aren't ready for merge can
always be deferred to a later release, but it seems weird to defer
drivers that are high quality just because they're submitted during
milestones 2 or 3.
"Crappy code" ... I don't know where that's coming from. If anything, CI has
helped get the drivers in faster to get rid of what you call "cramming".
That's good. If that's true, then I would think it supports an argument
that the deadlines are unnecessary because the underlying problem
(limited reviewer time) has been solved.
All the the above is just my opinion though, and you shouldn't care
about my opinions, as I don't do much coding and reviewing in Cinder.
There is a real reason I'm writing this email...
In Manila we added some major new features during Liberty. All of the
new features merged in the last week of L-3. It was a nightmare of
merge conflicts and angry core reviewers, and many contributors
worked through a holiday weekend to bring the release together. While
asking myself how we can avoid such a situation in the future, it
became clear to me that bigger features need to merge earlier -- the
earlier the better.
When I look at the release timeline, and ask myself when is the best
time to merge new major features, and when is the best time to merge
new drivers, it seems obvious that *features* need to happen early
and drivers should come *later*. New major features require FAR more
review time than new drivers, and they require testing, and even
after they merge they cause merge conflicts that everyone else has to
deal with. Better that that works happens in milestones 1 and 2 than
right before feature freeze. New drivers can come in right before
feature freeze as far as I'm concerned. Drivers don't cause merge
conflicts, and drivers don't need huge amounts of testing (presumably
the CI system ensure some level of quality).
It also occurs to me that new features which require driver
implementation (hello replication!) *really* should go in during the
first milestone so that drivers have time to implement the feature
during the same release.
I disagree. You're under the assumption that there is an intention of getting
a feature being worked in Liberty, to be ready for Liberty.
No.
I've expressed this numerous times at the Cinder midcycle sprint you attended
that I did not want to see drivers working on replication in their driver.
I'm all for features being worked on over multiple releases. I only
called out replication because of the recent controversy it caused. A
better example would have been a feature like "expand volume" which was
added some releases ago. It seems reasonable to me to add a feature like
that and for drivers to implement it all inside of one release. I'm not
sure what the next feature of that flavor will be, but whatever it is, I
hope it merges early in a release and not right before feature freeze.
So I'm asking the Cinder core team to reconsider the milestone-1
deadline for drivers, and to change it to a deadline for new major
features (in milestone-1 or milestone-2), and to allow drivers to
merge whenever*. This is the same pitch I'll be making to the Manila
core team. I've been considering this idea for a few weeks now but I
wanted to wait until after PTL elections to suggest it here.
During the release, a feature can be worked on, but adoption in drivers can be
difficult. Since just about every driver is behind on potential features, I'd
rather see driver maintainers focused on those features that have been ready
for sometime, not what we just merged a week ago.
There is no good reason to rush and suffer quality for the sake of vendors
wanting the latest and greatest feature. We're more mature than that.
Things like replication I'd rather see adopted in the next release. Otherwise,
if I can use you're own word, you're "cramming" features in a release.
[1] - https://etherpad.openstack.org/p/cinder-liberty-drivers
I hope you didn't read my post as a post from NetApp complaining about
drivers and driver features. I have no vested interest whatsoever in
what drivers get merged or what features get implemented. I'm writing
from my experience as Manila PTL and how the Manila core team got burned
by our own optimism regarding how many core features we could get into a
release.
Mike, I completely agree with you that vendor interests have to come
second to the project itself and I have no problem making vendors wait
to get their features in. I think we're even in vehement agreement and
you somehow assume I have a different motive. The balance of how much
time goes into reviewing vendor stuff vs core stuff is a hard problem
that I'm not addressing here, I'm ONLY trying to focus on the order in
which we do things, and I'm proposing that features should come first
and drivers should come last (in the release cycle).
-Ben Swartzlander
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
