Refactor PresubmitGuard updates to debounced asynchronous PubSub architecture

[eyebrowsoffire]

To reduce Firestore write contention when the status of many presubmit jobs update concurrently for a pull request, this change decouples PresubmitJob status updates from PresubmitGuard updates.

This addresses flutter/flutter#189161

Design Overview:

1. O(1) Job Completion (UnifiedCheckRun.markConclusion): When a presubmit job completes, markConclusion updates only the PresubmitJob document transactionally without querying or mutating the parent PresubmitGuard. This reduces write contention from O(N) jobs contending on a single PresubmitGuard document down to isolated O(1) transaction writes on each job document.

2. Atomic Debounced PubSub Triggering: After writing the PresubmitJob, markConclusion uses CacheService.setIfNotExists to atomically mark the PresubmitGuard as dirty (presubmit_guard_dirty) in Redis. If the flag is set (wasSet == true), it publishes a presubmit-guard-update message to PubSub. If another job completion occurs while the dirty flag is already present, setIfNotExists returns false and no duplicate PubSub message is published.

3. Asynchronous Guard Synchronization & Eventual Consistency: A new PresubmitGuardUpdateSubscription handler (/api/v2/presubmit-guard-update-subscription) receives the debounced PubSub message and delegates to Scheduler.processPresubmitGuardUpdate. Right before querying, the dirty flag is purged from Redis so subsequent job completions can re-dirty the cache and schedule another update. updatePresubmitGuard then queries the live PresubmitJob records, updates the PresubmitGuard document (remainingJobs, failedJobs, jobs), and evaluates any stage transitions (fusionEngineBuild, fusionTests, etc.) asynchronously. Reader endpoints (GetPresubmitGuard, GetPresubmitGuardSummaries, GetEngineArtifactsReady) continue to enjoy O(1) reads directly from the eventually consistent PresubmitGuard document.

[gemini-code-assist]

Code Review

This pull request introduces an asynchronous, debounced synchronization mechanism for PresubmitGuard state updates using Google Cloud Pub/Sub, moving away from synchronous transactional updates on every job completion to reduce database contention. It adds a new Pub/Sub subscription handler endpoint, updates the scheduler to process these updates asynchronously, and refactors UnifiedCheckRun to handle the split between marking job conclusions and updating guard status. Key feedback points out a potential lock-up issue if Pub/Sub publishing fails while the cache dirty flag remains set, a runtime type mismatch when passing TaskStatus enums instead of strings to PresubmitGuardStage, and opportunities to eliminate redundant Firestore reads by returning the updated guard directly from updatePresubmitGuard.

[ievdokdm]

Was this design agreed upon?
Do you have a bug or task associated with this implementation?
I'm working on another approach of introducing the ordered-presubmit-sub to process all notifications related to one check-run sequentially flutter/flutter#189029

[ievdokdm]

Is this implementation eventually call getLatestJobStatusesForGuard for every job status update ?

[eyebrowsoffire]

@ievdokdm

Was this design agreed upon?

I'm putting it out to review for discussion here. I started writing a doc, but ended up just making a PR to demonstrate the concept since easier to actually discuss concrete code and it wasn't that much work to actually write the implementation. It's basically the same as the more minimal change we had discussed last week on our GVC (separating the presubmit job update from the presubmit guard update and making the guard update asynchronous) except I just added a debouncing mechanism so that we don't need to update the guard for every job update if many of them come in quickly.

Do you have a bug or task associated with this implementation?

I filed flutter/flutter#189161

Is this implementation eventually call getLatestJobStatusesForGuard for every job status update ?

No, if there are a bunch of job updates in a short period, they all get debounced into a single pubsub message which updates the guard in one transaction.

[ievdokdm]

No, if there are a bunch of job updates in a short period, they all get debounced into a single pubsub message which updates the guard in one transaction.

but if updates come up, let say every 30 seconds for every of N jobs, are we going to call getLatestJobStatusesForGuard for every job update notification ?

[eyebrowsoffire]

@ievdokdm

but if updates come up, let say every 30 seconds for every of N jobs, are we going to call getLatestJobStatusesForGuard for every job update notification ?

Yes. However, looking at the Firestore usage insights, reads on these collections account for a miniscule portion of our firebase activity (0.0025% of read operations). Even in the worst case scenario, it's unlikely to have any significant impact on our firebase activity.

[ievdokdm]

@ievdokdm

but if updates come up, let say every 30 seconds for every of N jobs, are we going to call getLatestJobStatusesForGuard for every job update notification ?

Yes. However, looking at the Firestore usage insights, reads on these collections account for a miniscule portion of our firebase activity (0.0025% of read operations). Even in the worst case scenario, it's unlikely to have any significant impact on our firebase activity.

But we getting O(N^2) reads for every check run instead of O(N) that we have now. And with Message Ordering we would remain O(N) complexity and get rid of transaction collision.

[eyebrowsoffire]

But we getting O(N^2) reads for every check run instead of O(N) that we have now. And with flutter/flutter#189029 (comment) we would remain O(N) complexity and get rid of transaction collision.

Again, I don't think number of reads is a big concern here. The amount of reads used in either case is still insignificant. If you want to solve this with a different approach, that's fine, but this change is pretty simple and is basically what we discussed the other day (with an extra debouncing layer to avoid "thundering herd" issues). I don't know what you have in mind with the message ordering thing, since we didn't discuss that on our previous call.