-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[scheduler] Add auto-retries on pubsub notifications #1657
[scheduler] Add auto-retries on pubsub notifications #1657
Conversation
@@ -81,6 +88,20 @@ class PostsubmitLuciSubscription extends SubscriptionHandler { | |||
await datastore.insert(<Task>[task]); | |||
log.fine('Updated datastore'); | |||
|
|||
if (task.status == Task.statusFailed || task.status == Task.statusInfraFailure) { | |||
log.fine('Trying to auto-retry...'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also handle the manual retry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResetProdTask
is for manual retries, but it uses the same LuciBuildService.checkRerunBuilder
call (granted it can ignore the default checks)
AuthenticationProvider? authProvider, | ||
@visibleForTesting this.datastoreProvider = DatastoreService.defaultProvider, | ||
required this.luciBuildService, | ||
required this.scheduler, | ||
}) : super( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we using this topic luci-postsubmit
anywhere now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's used for targets that are using the cocoon scheduler (so cocoon, plugins, packages, and the engine's ci.yaml roller)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you point me to the locations referencing this topic? I am not able to find..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For scheduling post-submit builds, we add it here : https://cs.opensource.google/flutter/cocoon/+/main:app_dart/lib/src/service/luci_build_service.dart;l=503
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The topic here is luci-postsubmit
, where the referenced line uses topic luci-builds-prod
. Is this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's two parts to pubsub:
- Topics. This is generic, and requires a subscription
- Subscription. There can be N of these per topic to support multiple consumers
With that said, BuildBucket publishes the messages to the luci-postsubmit
topic, and Cocoon has a single subscription of luci-builds-prod
to get the messages. In the future, someone might add another subscription dedicated for the results processing aspect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes sense. Thanks for the explanations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
flutter/flutter#92300
This is dependent on #1656 (the test will pass after it lands)