Skip to content

Commit 2758690

Browse files
authored
Refactor instance creation: no need for microtask, or the finally block. (#9109)
1 parent 50b821e commit 2758690

File tree

1 file changed

+67
-70
lines changed

1 file changed

+67
-70
lines changed

app/lib/task/scheduler.dart

Lines changed: 67 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -169,78 +169,75 @@ Future<void> schedule(
169169
'package:${payload.package} analysis of ${payload.versions.length} '
170170
'versions.';
171171

172-
await Future.microtask(() async {
173-
var rollbackPackageState = true;
174-
try {
175-
// Purging cache is important for the edge case, where the new upload happens
176-
// on a different runtime version, and the current one's cache is still stale
177-
// and does not have the version yet.
178-
// TODO(https://github.com/dart-lang/pub-dev/issues/7268) remove after it gets fixed.
179-
await purgePackageCache(payload.package);
180-
_log.info(
181-
'creating instance $instanceName in $zone for '
182-
'package:${selected.package}',
183-
);
184-
await compute.createInstance(
185-
zone: zone,
186-
instanceName: instanceName,
187-
dockerImage: activeConfiguration.taskWorkerImage!,
188-
arguments: [json.encode(payload)],
189-
description: description,
190-
);
191-
rollbackPackageState = false;
192-
} on ZoneExhaustedException catch (e, st) {
193-
// A zone being exhausted is normal operations, we just use another
194-
// zone for 15 minutes.
195-
_log.info(
196-
'zone resources exhausted, banning ${e.zone} for 30 minutes',
197-
e,
198-
st,
199-
);
200-
// Ban usage of zone for 30 minutes
201-
banZone(e.zone, minutes: 30);
202-
} on QuotaExhaustedException catch (e, st) {
203-
// Quota exhausted, this can happen, but it shouldn't. We'll just stop
204-
// doing anything for 10 minutes. Hopefully that'll resolve the issue.
205-
// We log severe, because this is a reason to adjust the quota or
206-
// instance limits.
207-
_log.severe(
208-
'Quota exhausted trying to create $instanceName, banning all zones '
209-
'for 10 minutes',
210-
e,
211-
st,
212-
);
172+
var rollbackPackageState = true;
173+
try {
174+
// Purging cache is important for the edge case, where the new upload happens
175+
// on a different runtime version, and the current one's cache is still stale
176+
// and does not have the version yet.
177+
// TODO(https://github.com/dart-lang/pub-dev/issues/7268) remove after it gets fixed.
178+
await purgePackageCache(payload.package);
179+
_log.info(
180+
'creating instance $instanceName in $zone for '
181+
'package:${selected.package}',
182+
);
183+
await compute.createInstance(
184+
zone: zone,
185+
instanceName: instanceName,
186+
dockerImage: activeConfiguration.taskWorkerImage!,
187+
arguments: [json.encode(payload)],
188+
description: description,
189+
);
190+
rollbackPackageState = false;
191+
} on ZoneExhaustedException catch (e, st) {
192+
// A zone being exhausted is normal operations, we just use another
193+
// zone for 15 minutes.
194+
_log.info(
195+
'zone resources exhausted, banning ${e.zone} for 30 minutes',
196+
e,
197+
st,
198+
);
199+
// Ban usage of zone for 30 minutes
200+
banZone(e.zone, minutes: 30);
201+
} on QuotaExhaustedException catch (e, st) {
202+
// Quota exhausted, this can happen, but it shouldn't. We'll just stop
203+
// doing anything for 10 minutes. Hopefully that'll resolve the issue.
204+
// We log severe, because this is a reason to adjust the quota or
205+
// instance limits.
206+
_log.severe(
207+
'Quota exhausted trying to create $instanceName, banning all zones '
208+
'for 10 minutes',
209+
e,
210+
st,
211+
);
213212

214-
// Ban all zones for 10 minutes
215-
for (final zone in compute.zones) {
216-
banZone(zone, minutes: 10);
217-
}
218-
} on Exception catch (e, st) {
219-
// No idea what happened, but for robustness we'll stop using the zone
220-
// and shout into the logs
221-
_log.shout(
222-
'Failed to create instance $instanceName, banning zone "$zone" for '
223-
'15 minutes',
224-
e,
225-
st,
226-
);
227-
// Ban usage of zone for 15 minutes
228-
banZone(zone, minutes: 15);
229-
} finally {
230-
if (rollbackPackageState) {
231-
final oldVersionsMap = updated?.$2 ?? const {};
232-
// Restore the state of the PackageState for versions that were
233-
// suppose to run on the instance we just failed to create.
234-
// If this doesn't work, we'll eventually retry. Hence, correctness
235-
// does not hinge on this transaction being successful.
236-
await db.tasks.restorePreviousVersionsState(
237-
selected.package,
238-
instanceName,
239-
oldVersionsMap,
240-
);
241-
}
213+
// Ban all zones for 10 minutes
214+
for (final zone in compute.zones) {
215+
banZone(zone, minutes: 10);
242216
}
243-
});
217+
} on Exception catch (e, st) {
218+
// No idea what happened, but for robustness we'll stop using the zone
219+
// and shout into the logs
220+
_log.shout(
221+
'Failed to create instance $instanceName, banning zone "$zone" for '
222+
'15 minutes',
223+
e,
224+
st,
225+
);
226+
// Ban usage of zone for 15 minutes
227+
banZone(zone, minutes: 15);
228+
}
229+
if (rollbackPackageState) {
230+
final oldVersionsMap = updated?.$2 ?? const {};
231+
// Restore the state of the PackageState for versions that were
232+
// suppose to run on the instance we just failed to create.
233+
// If this doesn't work, we'll eventually retry. Hence, correctness
234+
// does not hinge on this transaction being successful.
235+
await db.tasks.restorePreviousVersionsState(
236+
selected.package,
237+
instanceName,
238+
oldVersionsMap,
239+
);
240+
}
244241
}
245242

246243
// Creating an instance can be slow, we want to schedule them concurrently.

0 commit comments

Comments
 (0)