workflows.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Han-Wen Nienhuys <hanwen@google.com>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
	Dmitry Vyukov <dvyukov@google.com>,
	Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	Laura Abbott <labbott@redhat.com>,
	Don Zickus <dzickus@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Daniel Axtens <dja@axtens.net>,
	David Miller <davem@davemloft.net>, Drew DeVault <sir@cmpwn.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	workflows@vger.kernel.org
Subject: Re: thoughts on a Merge Request based development workflow
Date: Tue, 15 Oct 2019 18:07:12 +0200	[thread overview]
Message-ID: <20191015160712.GD1003139@kroah.com> (raw)
In-Reply-To: <CAFQ2z_Pd2bSL+qpTNxwSNUOccvOt1QD9-XeCqqcdHtiNLKeJxA@mail.gmail.com>

On Mon, Oct 14, 2019 at 09:08:17PM +0200, Han-Wen Nienhuys wrote:
> (again, without html)
> 
> On Thu, Oct 10, 2019 at 11:00 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
> >
> > On Thu, Oct 10, 2019 at 07:52:50PM +0200, Dmitry Vyukov wrote:
> > > I know you all love Gerrit but just to clarify :)
> > > Gerrit stores all metadata in a git repo, all users can have a replica
> > > and you can always have, say, a "backup" replica on a side done
> > > automatically. Patches and versions of the patches are committed into
> > > git into special branches (e.g. change/XXX/version/YYY), comments and
> > > metadata are in a pretty straightforward json (this is comment text
> > > for line X, etc) also committed into git, so one can always read that
> > > in and transform into any other format. And you can also run Gerrit
> > > locally over your replica.
> >
> > Konstantin has spoken about some his concerns about git's scalability,
> > and it's important to remember that just because Gerrit has shown to
> > work well on some very large repositories, it doesn't necessarily mean
> > that it will work well on git repositories using the open source C
> > implementation of git.
> >
> > That's because Gerrit as used by Google (and made available in various
> > public-facing Gerrit servers) uses a Git-on-Borg implementation[1],
> > where the storage is done using Google's internal storage
> > infrastructure.  This is implemented on top of Jgit (which is git
> > implemented in Java)[2].
> >
> 
> 
> 
> Hi there,
> 
> I manage the Gerrit backend team at Google.
> 
> It is true that Gerrit at Google runs on top of Borg (Gerrit-on-Borg aka. GoB),
> 
> 1) there is no real concern about Cgit's scalability.
> 2) the borg deployment has no relevant magical sauce here.
> 
> To 1) : Konstantin was worried about performance implication on git
> notes.  The git-notes command stores data in a single
> refs/notes/commits branch. Gerrit actually uses notes (the file
> format) as well, but has a single notes branch per review, so
> performance here is not a concern when scaling up the number of
> reviews.
> 
> To 2) : Google needs special magic sauce, because we service hundreds
> of teams that work on thousands of repositories. However, here we're
> talking about just the kernel itself; that is just a single
> repository, and not an especially large one.  Chromium is our largest
> repo, and it is about 10x larger than the linux kernel.
> 
> Google runs gerrit in tasks with (currently) 16G memory each. There
> are many large companies (eg. SAP) that run much larger instances, ie.
> one can easily match GoB's performance level on a single machine.
> 
> I have been wanting to propose Gerrit as an alternative for the Linux
> kernel workflow, so I might as well bring forth my arguments here.
> 
> Gerrit isn't a big favorite of many people, but some of that
> perception may be outdated. Since 2016, Google has significantly
> increased its investment in Gerrit. For example, we have rewritten the
> web UI from scratch, and there have been many performance
> improvements.

As one of the many people who complain about Gerrit (I gave a whole talk
about it!), I guess I should comment here...

Yes, it's getting "better" for speed, but it's still way way too slow.

I don't think any of the complaints I gave many years ago have been
addressed, here's a few of them off the top of my head.

And note, I have a lot of experience using gerrit, I use it all the time
for Android kernel patchs, running your latest experimental version of
gerrit, so I assume that is what you are referring to when you say it is
under heavy development.  And I do like the changes you all are doing,
it is getting better in some ways (and worse in others, but that's for a
different thread...)

Anyway, my objections:
	- slow.  Seriously, have you tried using it on a slower network
	  connection (i.e. cellular teather, or train/bus wifi, or cafe
	  wifi?)
	- Can not see all changes made in a single commit across
	  multiple files on the same page.  You have to click through
	  each and every individual file to see the diffs.  That's
	  horrid for patches that touch multiple files and is my biggest
	  pet-peve about the tool.
	- patch series are a pain to apply, I have to manually open each
	  patch, give it a +2 and then when all of them are reviewed and
	  past testing, then they will be merged.  Is a pain.  Does no
	  one else accept patch series of 10-30 patches at a time other
	  than me?  How do they do it without opening 30+ tabs?

And, by reference of the "slow" issue, I should not have to do multiple
round-trip queries of a web interface just to see a single patch.
There's the initial cover page, then there's a click on each individual
file, bring up a new page for each and every file that was changed for
that commit to read it, and then when finished, clicking again to go
back to the initial page, and then a click to give a +2 and another
click to give a verified and then another refresh of the whole thing.

In contrast, try reading/reviewing and then applying a simple 10 patch
series from an email client like I do all the time.  The patches are
local, I read the whole diff with one button press (click if you have a
graphical email client), then if it is good, one keypress to apply it,
or save it to a mbox to apply them all at once later.

When you are reviewing thousands of patches a year, time matters.
Gerrit just does not cut it at all.  Remember, we only accept 1/3 of the
patches sent to us.  We are accepting 9 patches an hour, 24 hours a day.
That means we reject 18 patches an hour at that same time.

And then there's the issue of access when you do not have internet, like
I currently do not right now on a plane.  Or a very slow connection.  I
can still suck down patches in email apply them, and push them out.
Using gerrit on this connection is impossible.

> Building a review tool is not all that easy to do well; by using
> Gerrit, you get a tool that already exists, works, and has significant
> corporate support. We at Google have ~11 SWEs working on Gerrit
> full-time, for example, and we have support from UX research and UI
> design. The amount of work to tweak Gerrit for Linux kernel
> development surely is much less than building something from scratch.

I would love to see a better working gerrit today, for google, for the
developers there as that would save them time and energy that they
currently waste using it.  But for a distributed development / review
environment, with multiple people having multiple trees all over the
place, I don't know how Gerrit would work, unless it is trivial to
host/manage locally.

> Gerrit has a patchset oriented workflow (where changes are amended all
> the time), which is a good fit to the kernel's development process.

Maybe for some tiny subsystem's workflows, but not for any with a real
amount of development.  Try dumping a subsystem's patches into gerrit
today.  Can it handle something like netdev?  linux-input?  linux-usb?
staging?  Where does it start to break down in just being able to handle
the large quantities of changes?  Patchwork has done a lot to help some
of those subsystems work better, try seeing if gerrit could even handle
netdev in a sane manner.  Try to emulate what Dave does there, on your
own, and that should give you a huge idea of what you have to work on
already today, with gerrit, in order to make it better.

> Linus doesn't like Change-Id lines, but I think we could adapt Gerrit
> so it accepts URLs as IDs instead.

change-ids are the least of the problems of gerrit today :)

> There is talk of building a distributed/federated tool, but if there
> are policies ("Jane Doe is maintainer of the network subsystem, and
> can merge changes that only touch file in net/ "), then building
> something decentralized is really hard. You have to build
> infrastructure where Jane can prove to others who she is (PGP key
> signing parties?), and some sort of distributed storage of the policy
> rules.
> 
> By contrast, a centralized server can authenticate users reliably and
> the server owner can define such rules.  There can still be multiple
> gerrit servers, possibly sponsored by corporate entities (one from
> RedHat, one from Google, etc.), and different servers can support
> different authentication models (OpenID, OAuth, Google account, etc.)

Like Daniel said, the kernel is multi-repos for a mono-tree.  I don't
think Gerrit is set up to handle that at all from what I can see.

How many people does it take to maintain an Gerrit instance and keep it
up and running well?

thanks,

greg k-h

  parent reply	other threads:[~2019-10-15 16:07 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-24 18:25 thoughts on a Merge Request based development workflow Neil Horman
2019-09-24 18:37 ` Drew DeVault
2019-09-24 18:53   ` Neil Horman
2019-09-24 20:24     ` Laurent Pinchart
2019-09-24 22:25       ` Neil Horman
2019-09-25 20:50         ` Laurent Pinchart
2019-09-25 21:54           ` Neil Horman
2019-09-26  0:40           ` Neil Horman
2019-09-28 22:58             ` Steven Rostedt
2019-09-28 23:16               ` Dave Airlie
2019-09-28 23:52                 ` Steven Rostedt
2019-10-01  3:22                 ` Daniel Axtens
2019-10-01 21:14                   ` Bjorn Helgaas
2019-09-29 11:57               ` Neil Horman
2019-09-29 12:55                 ` Dmitry Vyukov
2019-09-30  1:00                   ` Neil Horman
2019-09-30  6:05                     ` Dmitry Vyukov
2019-09-30 12:55                       ` Neil Horman
2019-09-30 13:20                         ` Nicolas Belouin
2019-09-30 13:40                         ` Dmitry Vyukov
2019-09-30 21:02                     ` Konstantin Ryabitsev
2019-09-30 14:51                   ` Theodore Y. Ts'o
2019-09-30 15:15                     ` Steven Rostedt
2019-09-30 16:09                       ` Geert Uytterhoeven
2019-09-30 20:56                       ` Konstantin Ryabitsev
2019-10-08  1:00                     ` Stephen Rothwell
2019-09-26 10:23           ` Geert Uytterhoeven
2019-09-26 13:43             ` Neil Horman
2019-10-07 15:33   ` David Miller
2019-10-07 15:35     ` Drew DeVault
2019-10-07 16:20       ` Neil Horman
2019-10-07 16:24         ` Drew DeVault
2019-10-07 18:43           ` David Miller
2019-10-07 19:24             ` Eric Wong
2019-10-07 15:47     ` Steven Rostedt
2019-10-07 18:40       ` David Miller
2019-10-07 18:45       ` David Miller
2019-10-07 19:21         ` Steven Rostedt
2019-10-07 21:49     ` Theodore Y. Ts'o
2019-10-07 23:00     ` Daniel Axtens
2019-10-08  0:39       ` Eric Wong
2019-10-08  1:26         ` Daniel Axtens
2019-10-08  2:11           ` Eric Wong
2019-10-08  3:24             ` Daniel Axtens
2019-10-08  6:03               ` Eric Wong
2019-10-08 10:06                 ` Daniel Axtens
2019-10-08 13:19                   ` Steven Rostedt
2019-10-08 18:46                 ` Rob Herring
2019-10-08 21:36                   ` Eric Wong
2019-10-08  1:17       ` Steven Rostedt
2019-10-08 16:43         ` Don Zickus
2019-10-08 17:17           ` Steven Rostedt
2019-10-08 17:39             ` Don Zickus
2019-10-08 19:05               ` Konstantin Ryabitsev
2019-10-08 20:32                 ` Don Zickus
2019-10-08 21:35                   ` Konstantin Ryabitsev
2019-10-09 21:50                     ` Laura Abbott
2019-10-10 12:48                       ` Neil Horman
2019-10-09 21:35                 ` Laura Abbott
2019-10-09 21:54                   ` Konstantin Ryabitsev
2019-10-09 22:09                     ` Laura Abbott
2019-10-09 22:19                       ` Dave Airlie
2019-10-09 22:21                     ` Eric Wong
2019-10-09 23:56                       ` Konstantin Ryabitsev
2019-10-10  0:07                         ` Eric Wong
2019-10-10  7:35                         ` Nicolas Belouin
2019-10-10 12:53                           ` Steven Rostedt
2019-10-10 14:21                           ` Dmitry Vyukov
2019-10-11  7:12                             ` Nicolas Belouin
2019-10-11 13:56                               ` Dmitry Vyukov
2019-10-14  7:31                                 ` Nicolas Belouin
2019-10-10 17:52                     ` Dmitry Vyukov
2019-10-10 20:57                       ` Theodore Y. Ts'o
2019-10-11 11:01                         ` Dmitry Vyukov
2019-10-11 12:54                           ` Theodore Y. Ts'o
2019-10-14 19:08                         ` Han-Wen Nienhuys
2019-10-15  1:54                           ` Theodore Y. Ts'o
2019-10-15 12:00                             ` Daniel Vetter
2019-10-15 13:14                               ` Han-Wen Nienhuys
2019-10-15 13:45                                 ` Daniel Vetter
2019-10-16 18:56                                   ` Han-Wen Nienhuys
2019-10-16 19:08                                     ` Mark Brown
2019-10-17 10:22                                       ` Han-Wen Nienhuys
2019-10-17 11:24                                         ` Mark Brown
2019-10-17 11:49                                     ` Daniel Vetter
2019-10-17 12:09                                       ` Han-Wen Nienhuys
2019-10-17 12:53                                         ` Daniel Vetter
2019-10-15 16:07                           ` Greg KH [this message]
2019-10-15 16:35                             ` Steven Rostedt
2019-10-15 18:58                             ` Han-Wen Nienhuys
2019-10-15 19:33                               ` Greg KH
2019-10-15 20:03                                 ` Mark Brown
2019-10-15 19:50                               ` Mark Brown
2019-10-15 18:37                           ` Konstantin Ryabitsev
2019-10-15 19:15                             ` Han-Wen Nienhuys
2019-10-15 19:35                               ` Greg KH
2019-10-15 19:41                               ` Konstantin Ryabitsev
2019-10-16 18:33                                 ` Han-Wen Nienhuys
2019-10-09  2:02           ` Daniel Axtens
2019-09-24 23:15 ` David Rientjes
2019-09-25  6:35   ` Toke Høiland-Jørgensen
2019-09-25 10:49   ` Neil Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191015160712.GD1003139@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=dja@axtens.net \
    --cc=dvyukov@google.com \
    --cc=dzickus@redhat.com \
    --cc=hanwen@google.com \
    --cc=konstantin@linuxfoundation.org \
    --cc=labbott@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=rostedt@goodmis.org \
    --cc=sir@cmpwn.com \
    --cc=tytso@mit.edu \
    --cc=workflows@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).