Skip to content

Commit 0e06015

Browse files
isoosisoos
committed
Refactor instance creation: no need for microtask, or the finally block. (#9109)
1 parent 9f7ab5f commit 0e06015

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
@@ -130,78 +130,75 @@ Future<void> schedule(
130130
'package:${payload.package} analysis of ${payload.versions.length} '
131131
'versions.';
132132

133-
await Future.microtask(() async {
134-
var rollbackPackageState = true;
135-
try {
136-
// Purging cache is important for the edge case, where the new upload happens
137-
// on a different runtime version, and the current one's cache is still stale
138-
// and does not have the version yet.
139-
// TODO(https://github.com/dart-lang/pub-dev/issues/7268) remove after it gets fixed.
140-
await purgePackageCache(payload.package);
141-
_log.info(
142-
'creating instance $instanceName in $zone for '
143-
'package:${selected.package}',
144-
);
145-
await compute.createInstance(
146-
zone: zone,
147-
instanceName: instanceName,
148-
dockerImage: activeConfiguration.taskWorkerImage!,
149-
arguments: [json.encode(payload)],
150-
description: description,
151-
);
152-
rollbackPackageState = false;
153-
} on ZoneExhaustedException catch (e, st) {
154-
// A zone being exhausted is normal operations, we just use another
155-
// zone for 15 minutes.
156-
_log.info(
157-
'zone resources exhausted, banning ${e.zone} for 30 minutes',
158-
e,
159-
st,
160-
);
161-
// Ban usage of zone for 30 minutes
162-
banZone(e.zone, minutes: 30);
163-
} on QuotaExhaustedException catch (e, st) {
164-
// Quota exhausted, this can happen, but it shouldn't. We'll just stop
165-
// doing anything for 10 minutes. Hopefully that'll resolve the issue.
166-
// We log severe, because this is a reason to adjust the quota or
167-
// instance limits.
168-
_log.severe(
169-
'Quota exhausted trying to create $instanceName, banning all zones '
170-
'for 10 minutes',
171-
e,
172-
st,
173-
);
133+
var rollbackPackageState = true;
134+
try {
135+
// Purging cache is important for the edge case, where the new upload happens
136+
// on a different runtime version, and the current one's cache is still stale
137+
// and does not have the version yet.
138+
// TODO(https://github.com/dart-lang/pub-dev/issues/7268) remove after it gets fixed.
139+
await purgePackageCache(payload.package);
140+
_log.info(
141+
'creating instance $instanceName in $zone for '
142+
'package:${selected.package}',
143+
);
144+
await compute.createInstance(
145+
zone: zone,
146+
instanceName: instanceName,
147+
dockerImage: activeConfiguration.taskWorkerImage!,
148+
arguments: [json.encode(payload)],
149+
description: description,
150+
);
151+
rollbackPackageState = false;
152+
} on ZoneExhaustedException catch (e, st) {
153+
// A zone being exhausted is normal operations, we just use another
154+
// zone for 15 minutes.
155+
_log.info(
156+
'zone resources exhausted, banning ${e.zone} for 30 minutes',
157+
e,
158+
st,
159+
);
160+
// Ban usage of zone for 30 minutes
161+
banZone(e.zone, minutes: 30);
162+
} on QuotaExhaustedException catch (e, st) {
163+
// Quota exhausted, this can happen, but it shouldn't. We'll just stop
164+
// doing anything for 10 minutes. Hopefully that'll resolve the issue.
165+
// We log severe, because this is a reason to adjust the quota or
166+
// instance limits.
167+
_log.severe(
168+
'Quota exhausted trying to create $instanceName, banning all zones '
169+
'for 10 minutes',
170+
e,
171+
st,
172+
);
174173

175-
// Ban all zones for 10 minutes
176-
for (final zone in compute.zones) {
177-
banZone(zone, minutes: 10);
178-
}
179-
} on Exception catch (e, st) {
180-
// No idea what happened, but for robustness we'll stop using the zone
181-
// and shout into the logs
182-
_log.shout(
183-
'Failed to create instance $instanceName, banning zone "$zone" for '
184-
'15 minutes',
185-
e,
186-
st,
187-
);
188-
// Ban usage of zone for 15 minutes
189-
banZone(zone, minutes: 15);
190-
} finally {
191-
if (rollbackPackageState) {
192-
final oldVersionsMap = updated?.$2 ?? const {};
193-
// Restore the state of the PackageState for versions that were
194-
// suppose to run on the instance we just failed to create.
195-
// If this doesn't work, we'll eventually retry. Hence, correctness
196-
// does not hinge on this transaction being successful.
197-
await db.tasks.restorePreviousVersionsState(
198-
selected.package,
199-
instanceName,
200-
oldVersionsMap,
201-
);
202-
}
174+
// Ban all zones for 10 minutes
175+
for (final zone in compute.zones) {
176+
banZone(zone, minutes: 10);
203177
}
204-
});
178+
} on Exception catch (e, st) {
179+
// No idea what happened, but for robustness we'll stop using the zone
180+
// and shout into the logs
181+
_log.shout(
182+
'Failed to create instance $instanceName, banning zone "$zone" for '
183+
'15 minutes',
184+
e,
185+
st,
186+
);
187+
// Ban usage of zone for 15 minutes
188+
banZone(zone, minutes: 15);
189+
}
190+
if (rollbackPackageState) {
191+
final oldVersionsMap = updated?.$2 ?? const {};
192+
// Restore the state of the PackageState for versions that were
193+
// suppose to run on the instance we just failed to create.
194+
// If this doesn't work, we'll eventually retry. Hence, correctness
195+
// does not hinge on this transaction being successful.
196+
await db.tasks.restorePreviousVersionsState(
197+
selected.package,
198+
instanceName,
199+
oldVersionsMap,
200+
);
201+
}
205202
}
206203

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

0 commit comments

Comments
 (0)