ListAllJobs endpoint can be watched for changes

[aoldershaw]

What does this PR accomplish?

Bug Fix | Feature | Documentation

Related RFC: concourse/rfcs#61

Changes proposed by this PR:

This PR aims to provide functionality similar to Kubernetes' ?watch API (e.g. kubectl get ... --watch) for the ListAllJobs endpoint.

The motivation for this is described more in depth in the RFC, but essentially, it should provide:

• (Hopefully) better performance on the ATC and DB
• Faster feedback in the UI dashboard

Eventually, I see this change applying to several endpoints - rather than polling in the UI, we have the ATC send us updates.

Notes to reviewer:

To test: visit the dashboard, and then modify some jobs! (e.g. trigger builds, set/delete pipelines, expose/hide pipelines, etc)

This is feature flagged behind the --enable-watch-endpoints flag, which I've enabled by default in docker-compose.yml.

Contributor Checklist

• Followed Code of conduct, Contributing Guide & avoided Anti-patterns
• Signed all commits
• Added tests (Unit and/or Integration)
• Updated Documentation
• Updated Release notes

Reviewer Checklist

• Code reviewed
• Tests reviewed
• Documentation reviewed
• Release notes reviewed
• PR acceptance performed
• New config flags added? Ensure that they are added to the
  BOSH and
  Helm packaging; otherwise, ignored for
  the integration
  tests
  (for example, if they are Garden configs that are not displayed in the
  --help text).

[jamieklassen]

I have come back to this several times and never really convinced myself I understood how it works. The subtleties of queuing/not queuing notifications and handling dropped/interrupted connections or other edge-casey things seem like they would be easy to get wrong, and I couldn't grasp what dirty meant. If I was designing something like this from the ground up I'm sure I'd follow the example of kubernetes and use something like etcd which already has a "watch" system.

However, I don't want to block this potentially-valuable performance improvement just because my brain is too small. The feature flag seems to work properly so this is totally backwards-compatible, so I'd rather focus on getting it merged and load-tested instead of staring at it forever. So let's get it rebased, add the feature flag into the helm chart and bosh release, and turn it on in hush-house.

Maybe we can focus some effort on planning the load test experiment - what will we measure (memory on the web nodes, network i/o between the DB and web, number of DB connections? probably something else on the DB)? what do we expect to change? this exercise might motivate us to add some observability. A gauge for the number of open ListAllJobs connections might not be amiss.

[aoldershaw]

@jamieklassen thanks for the review! Hmm, I think if you are struggling to fully understand it then there's definitely a need to improve upon the clarity - I'll try to think about that before merging.

I'd also like to a load test experiment as you suggested, but haven't found the time. I think it's probably wise to validate the impact it has before adding so much more complexity to the codebase

[aoldershaw]

Opened #6084 to track load testing

[aoldershaw]

@jamieklassen I've done some load testing and documented it here: #6084

The skinny is that, under heavy load (tested with 100 active "dashboards"), enabling watch endpoints seems to result in:

• Reduced ATC CPU usage, and as a result, increased build scheduling (compared to when watch endpoints are disabled)
  • Without watch endpoints, the ATCs CPU was pegged at 100% (compared to 80% in the baseline), and build scheduling dropped by ~50% as result
  • With watch endpoints, the ATCs CPU was around 85-95% (compared to 80% in the baseline), and build scheduling only dropped by ~10%
• Reduced network transmission (ATC -> browser dropped by ~70%, DB -> ATC dropped by ~60%)
• Slightly increased memory usage on both ATC/DB (~10-15%)

Not under client load (just the regular build scheduling load), having watch endpoints enabled resulted in slightly higher DB CPU and memory (~5-8% for each metric) - worth noting that at least some of this could be ascribed to the fact that the watch endpoint test was run after the vanilla concourse test, so the magnitude of data in the DB was a bit larger. The completed DB transaction rate didn't seem to be significantly affected, nor did DB connections.

---

In doing the load testing, I noticed some performance issues on the UI, primarily caused by writing the jobs cache to localStorage after every batch of events. I added some optimizations to mitigate that (9af0bc7, 100e067).

this exercise might motivate us to add some observability. A gauge for the number of open ListAllJobs connections might not be amiss

While looking into this, I noticed we have a GaugeVec for concurrent requests as part of the concurrent request limiting you worked on. I wonder if we should just always emit the number of ListAllJobs concurrent requests even if there is no concurrent request limit set.

It also makes me wonder about how the watch endpoints should play with the concurrent request limit - it's no longer a "you'll get your turn eventually, so just keep retrying" sort of thing. What do you think?

[chenbh]

Closing as it looks like we won't be prioritizing this anytime soon and the merge conflicts look scary