another offline conversation with iannucci and hinoka

we still need the builder summary transaction because pubsub message order is not necessarily the same as build completion order. However, this is fine because we will datastore.get of builder summary only if the build is completed and we will do datastore.put only if the build is newer than whatever the got builder summary has right now. Then the frequency of builder summary datastore ops == build completion frequency in a builder, which is small enough to not to cause datastore collision.

It is possible that same pubsub messages are duped, so we may receive multiple "build is completed" messages. To mitigate that, when pubsub handler updates BuildSummary, it should check if BuildSummary is already terminal, in which case the handler should just exit.

I kinda suspect that we'll still want Consoles in there, though maybe we can update that out of band (i.e. when we actually ingest the console configs). The reason for this is that on the builder/builders page, it would be pretty awesome to display links to every console containing the builder.

That said, we can always add it back in when we implement such a feature :)

FWIW, I just uploaded a refactor which makes populating Consoles free: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/759917 

it was not free before? it wasn't expensive, it is just being updated in a wrong place. if there are no builds coming, it is not updated

It was... sort-of incidentally free. It relied on external code updating the BuildSummary in a certain way. The CL above makes it non-abstraction-violating.

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/803248fb69927c89eeabf4eca697db2447185df3

commit 803248fb69927c89eeabf4eca697db2447185df3
Author: Robert Iannucci <iannucci@chromium.org>
Date: Thu Nov 09 08:20:59 2017

[milo] Console caching cleanup.

  * Adds per-request caching for common.GetAllConsoles()
  * Removes ad-hoc `BuildSummary.consoles` cache thingy
  * Simplifies manifest key interface
  * Makes BuilderSummary extract console IDs from ManifestKeys
  * Parallelizes all tests
  * Fix test inter-dependency by moving context initialization inside
    Convey (previously tests were persisting state between tests).

R=hinoka@chromium.org, jchinlee@chromium.org, nodir@chromium.org, tandrii@chromium.org

Bug:  782489 ,  782874 
Change-Id: I8c84482a7b0f6f8584c86349872cc26546432527
Reviewed-on: https://chromium-review.googlesource.com/759917
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbot/build_test.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbot/builder_test.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbot/buildinfo_test.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbot/buildstore/build.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbot/grpc_test.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbot/pubsub_test.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbucket/pubsub.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/buildsource/buildbucket/pubsub_test.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/common/config.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/common/model/build_summary.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/common/model/builder_summary.go
[modify] https://crrev.com/803248fb69927c89eeabf4eca697db2447185df3/milo/common/model/builder_summary_test.go

Add labels. This is a P0 for migration. 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/e0239b3d14161869fddccf55fe8df294d5dd69f8

commit e0239b3d14161869fddccf55fe8df294d5dd69f8
Author: Ryan Tseng <hinoka@chromium.org>
Date: Thu Nov 09 20:58:44 2017

Revert "[milo] Console caching cleanup."

This reverts commit 803248fb69927c89eeabf4eca697db2447185df3.

Reason for revert: Speculative revert: Console broken because manifest keys aren't getting populated, and BuildSummaries aren't working

Original change's description:
> [milo] Console caching cleanup.
> 
>   * Adds per-request caching for common.GetAllConsoles()
>   * Removes ad-hoc `BuildSummary.consoles` cache thingy
>   * Simplifies manifest key interface
>   * Makes BuilderSummary extract console IDs from ManifestKeys
>   * Parallelizes all tests
>   * Fix test inter-dependency by moving context initialization inside
>     Convey (previously tests were persisting state between tests).
> 
> R=​hinoka@chromium.org, jchinlee@chromium.org, nodir@chromium.org, tandrii@chromium.org
> 
> Bug:  782489 ,  782874 
> Change-Id: I8c84482a7b0f6f8584c86349872cc26546432527
> Reviewed-on: https://chromium-review.googlesource.com/759917
> Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
> Reviewed-by: Nodir Turakulov <nodir@chromium.org>

TBR=iannucci@chromium.org,hinoka@chromium.org,nodir@chromium.org,jchinlee@chromium.org,tandrii@chromium.org

Change-Id: I1e24cc7e6c0cc714ed9d1f1f3dba11a3a27d6767
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  782489 ,  782874 
Reviewed-on: https://chromium-review.googlesource.com/761438
Commit-Queue: Ryan Tseng <hinoka@chromium.org>
Reviewed-by: Ryan Tseng <hinoka@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbot/build_test.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbot/builder_test.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbot/buildinfo_test.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbot/buildstore/build.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbot/grpc_test.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbot/pubsub_test.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbucket/pubsub.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/buildsource/buildbucket/pubsub_test.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/common/config.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/common/model/build_summary.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/common/model/builder_summary.go
[modify] https://crrev.com/e0239b3d14161869fddccf55fe8df294d5dd69f8/milo/common/model/builder_summary_test.go

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/ae4d8b55e16a7c963626e2629a626d25fdfc0082

commit ae4d8b55e16a7c963626e2629a626d25fdfc0082
Author: Ryan Tseng <hinoka@google.com>
Date: Wed Nov 15 00:52:09 2017

[milo] Load console summaries with direct datastore fetches.

Instead of using the console index.

Bug:  782874 
Change-Id: If49a3c0942c62fbefb2bc6f2d9fd6dfc20d21263
Reviewed-on: https://chromium-review.googlesource.com/765171
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Ryan Tseng <hinoka@chromium.org>

[modify] https://crrev.com/ae4d8b55e16a7c963626e2629a626d25fdfc0082/milo/buildsource/console.go
[modify] https://crrev.com/ae4d8b55e16a7c963626e2629a626d25fdfc0082/milo/frontend/ui/console.go

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/0451818df7d38a8583613b2519079b13834ead86

commit 0451818df7d38a8583613b2519079b13834ead86
Author: Nodir Turakulov <nodir@google.com>
Date: Wed Nov 15 22:44:55 2017

[milo] Cleanup BuilderSummary

Context:  crbug.com/782874 

Remove Console and InProgress fields from BuilderSummary
and make transactions in build pubsub pipeine not XG.

Also use transactions at config ingestion time.

Bug:  782874 
Change-Id: I92ec7a2a1737bfb6af885814b24a18003ee05996
Reviewed-on: https://chromium-review.googlesource.com/772973
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Jao-ke Chin-Lee <jchinlee@chromium.org>

[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/buildsource/buildbot/buildstore/build.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/buildsource/buildbot/grpc_test.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/buildsource/buildbot/pubsub_test.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/buildsource/buildbucket/pubsub.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/buildsource/buildbucket/pubsub_test.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/common/config.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/common/model/builder_summary.go
[modify] https://crrev.com/0451818df7d38a8583613b2519079b13834ead86/milo/common/model/builder_summary_test.go

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/b64efee779695c97da94caa36f9bc6817f482382

commit b64efee779695c97da94caa36f9bc6817f482382
Author: Nodir Turakulov <nodir@google.com>
Date: Tue Nov 28 02:11:27 2017

[milo] remove BuilderSummary.Console

https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/772973
intended to delete BuilderSummary.Console field. It removes its usages, but
did not remove the field itself. Remove it

Bug:  782874 
Change-Id: I69ca043567a4305b4028d75e962f6372cdeeff5c
Reviewed-on: https://chromium-review.googlesource.com/792533
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/b64efee779695c97da94caa36f9bc6817f482382/milo/common/model/builder_summary.go