> So I propose to add before scheduling a build a check if a build is
> already running for the same thing as the current event, and if it is
> don't schedule a build.  That would take care of multiple builds
> running at the same time, and if a new job is configured in between it
> would just be triggered and added to the same memory imprint.  To
> accomplish that we would need to have the key in the build memory
> based on something that can be calculated like event type + patch set
> and if needed something special for that event type.  Then we would
> have coordination for each event type and patch set, and maybe that is
> good enough?

Our primary concern is that we should never run a partial set of
configured tests run for an event.  I think its okay to skip running an
entire set of tests if there is still an ongoing build event, which I
believe is what you were saying.  So if Event A triggers some builds and
an identical Event B arrives while at least one of them is still
running, no additional builds are triggered.  And if a new job is
configured in the intervening time, it _should_ be triggered, but added
to the build memory imprint of the original event.

Here's a visual timeline:
   http://paste.openstack.org/show/6883/

> One problem is with the CommentAdded type itself, I guess we want to
> differentiate between the different approval types (code review,
> verified, etc.) and people can set all of them in the same comment I
> guess, could we split those up into separate events internally?

This sounds reasonable.  The second event may end up overwriting the
results of the first, but that can happen if two people each leave votes
in the different categories near the same time, so the Jenkins
administrator should already be planning on that.  Presumably if you're
listening to different events, you want to perform different actions as
a result (perhaps vote in a different category, or not vote at all).

> And if, in the example above, someone three days later add another +2
> all jobs would trigger again and overwrite the old result. But I guess
> that is a non issue, or something that we can live with?

It's very much desired in our case.  Many of our jobs fail due to
outside forces, so being able to retrigger the full set of jobs (either
immediately after the run or later) is required.

> Another scenario that complicates things: Lets say an upload triggers
> project X and Y. While they are building someone makes a comment that
> triggers project A and B.  X and Y finishes with -1 and later A and B
> finishes with +1 overwriting the Jenkins verdict in Gerrit.
>
> I think that if we just make people aware of this limitation we could
> live with it, i.e. propose a workflow that states "don't do anything
> that would trigger a new set of builds untill you have your +1" It's a
> little lame to say so, but it is something that we can work on in
> later features to fix.
>
> What do you think?

I think this behavior is okay.  In our setup, we have a subset of jobs
run on patchset-uploaded, and the full set run on APRV+1.  If someone
approves before the patchset-uploaded jobs are finished, that's fine.
The approval jobs will run (and take longer since they are a superset)
and overwrite the patchset-uploaded vote.

I think if you have completely independent sets of jobs, as in your
example, you will probably want the results to do something different,
like vote in a different category.  We may need to eventually expand how
the trigger plugin votes to make it more configurable, but I don't think
that's needed for what we have already to start being useful.

I haven't had time to implement the above changes, and probably won't at
least for a little while.  As I mentioned earlier, our most important
concern on the OpenStack project is that the correct jobs get run at
least once, not necessarily that they run most efficiently (without
duplication).  We've been using a branch of this code with ref-updated
added in production for about a month.  I think we should close this
pull request, and I'll open a new one for the ref-updated branch, and we
can proceed from there.