fix(subsUnion): stop treating a failed worker call as a valid empty union
All checks were successful
build-image / build (push) Successful in 12m16s
All checks were successful
build-image / build (push) Successful in 12m16s
storeUnion checked `typeof union === 'object'` as its "did this work" guard, but typeof null is also 'object' — so a worker call that failed (caught upstream, leaving union as its initial null) looked identical to a legitimate "zero valid subscriptions" result, and got silently stored as the string "null" instead of being treated as a failure. This is exactly what corrupted subs-public-union/subs-private-union on staging just now, while chasing the missing-turf-deps bug. storeUnion is now only called once calcUnionAsync has actually resolved; process() and incrementalAdd() catch failures themselves and skip storing anything, leaving the previous (still valid) union alone.
This commit is contained in:
parent
4879d3482d
commit
50ca9cc6cf
1 changed files with 47 additions and 47 deletions
|
|
@ -41,48 +41,50 @@ Meteor.startup(async () => {
|
|||
return valid;
|
||||
};
|
||||
|
||||
// Only called once calcUnionAsync has actually resolved — union === null here
|
||||
// means "zero valid subscriptions", a real result, not a failed computation.
|
||||
// (A prior version guarded this with `typeof union === 'object'`, which is
|
||||
// true for null too, so a failed worker call — caught upstream, leaving
|
||||
// union as null — looked identical to a legitimate empty union and got
|
||||
// silently stored as "null". Callers must catch calcUnionAsync failures
|
||||
// themselves and skip calling this, instead of relying on this check.)
|
||||
const storeUnion = async (isPublic, union, count) => {
|
||||
const publicl = isPublic ? 'public' : 'private';
|
||||
const Publicl = publicl.replace(/\b\w/g, l => l.toUpperCase());
|
||||
|
||||
if (typeof union === 'object') {
|
||||
const bounds = union === null ? null : L.geoJSON(union).getBounds();
|
||||
const unionSet = {
|
||||
$set: {
|
||||
name: `subs-${publicl}-union`,
|
||||
value: JSON.stringify(union),
|
||||
isPublic,
|
||||
description: `${Publicl} subscriptions union`,
|
||||
type: 'string'
|
||||
}
|
||||
};
|
||||
const boundsSet = {
|
||||
$set: {
|
||||
name: `subs-${publicl}-union-bounds`,
|
||||
value: JSON.stringify(bounds),
|
||||
isPublic,
|
||||
description: `${Publicl} subscriptions union bounds`,
|
||||
type: 'string'
|
||||
}
|
||||
};
|
||||
const sizeSet = {
|
||||
$set: {
|
||||
name: 'subs-union-count',
|
||||
value: count,
|
||||
isPublic: false,
|
||||
description: 'Subscriptions count',
|
||||
type: 'number'
|
||||
}
|
||||
};
|
||||
// FIXME, take care of object size:
|
||||
// https://stackoverflow.com/questions/10827812/what-is-the-length-maximum-for-a-string-data-type-in-mongodb-used-with-ruby
|
||||
await SiteSettings.upsertAsync({ name: `subs-${publicl}-union` }, unionSet, { multi: false });
|
||||
await SiteSettings.upsertAsync({ name: `subs-${publicl}-union-bounds` }, boundsSet, { multi: false });
|
||||
await SiteSettings.upsertAsync({ name: 'subs-union-count' }, sizeSet, { multi: false });
|
||||
if (debug) console.log(`${Publicl} subscription union calculated`);
|
||||
} else {
|
||||
console.log('Subscription union failed!');
|
||||
}
|
||||
const bounds = union === null ? null : L.geoJSON(union).getBounds();
|
||||
const unionSet = {
|
||||
$set: {
|
||||
name: `subs-${publicl}-union`,
|
||||
value: JSON.stringify(union),
|
||||
isPublic,
|
||||
description: `${Publicl} subscriptions union`,
|
||||
type: 'string'
|
||||
}
|
||||
};
|
||||
const boundsSet = {
|
||||
$set: {
|
||||
name: `subs-${publicl}-union-bounds`,
|
||||
value: JSON.stringify(bounds),
|
||||
isPublic,
|
||||
description: `${Publicl} subscriptions union bounds`,
|
||||
type: 'string'
|
||||
}
|
||||
};
|
||||
const sizeSet = {
|
||||
$set: {
|
||||
name: 'subs-union-count',
|
||||
value: count,
|
||||
isPublic: false,
|
||||
description: 'Subscriptions count',
|
||||
type: 'number'
|
||||
}
|
||||
};
|
||||
// FIXME, take care of object size:
|
||||
// https://stackoverflow.com/questions/10827812/what-is-the-length-maximum-for-a-string-data-type-in-mongodb-used-with-ruby
|
||||
await SiteSettings.upsertAsync({ name: `subs-${publicl}-union` }, unionSet, { multi: false });
|
||||
await SiteSettings.upsertAsync({ name: `subs-${publicl}-union-bounds` }, boundsSet, { multi: false });
|
||||
await SiteSettings.upsertAsync({ name: 'subs-union-count' }, sizeSet, { multi: false });
|
||||
if (debug) console.log(`${Publicl} subscription union calculated`);
|
||||
};
|
||||
|
||||
const process = async (isPublic) => {
|
||||
|
|
@ -90,13 +92,12 @@ Meteor.startup(async () => {
|
|||
const decorate = isPublic ? addNoisy : noNoisy;
|
||||
const decorated = subscribers.filter(isValidSub).map(decorate);
|
||||
|
||||
let union = null;
|
||||
try {
|
||||
union = await calcUnionAsync(decorated);
|
||||
const union = await calcUnionAsync(decorated);
|
||||
await storeUnion(isPublic, union, subscribers.length);
|
||||
} catch (e) {
|
||||
console.error('subsUnion worker failed', e);
|
||||
console.error('subsUnion worker failed, union left unchanged', e);
|
||||
}
|
||||
await storeUnion(isPublic, union, subscribers.length);
|
||||
};
|
||||
|
||||
const recreate = async () => { await process(true); await process(false); };
|
||||
|
|
@ -138,17 +139,16 @@ Meteor.startup(async () => {
|
|||
const current = await SiteSettings.findOneAsync({ name: `subs-${publicl}-union` });
|
||||
const baseUnion = current && current.value ? JSON.parse(current.value) : null;
|
||||
|
||||
let union = null;
|
||||
try {
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
union = await calcUnionAsync([decorated], baseUnion);
|
||||
const union = await calcUnionAsync([decorated], baseUnion);
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
await storeUnion(isPublic, union, countSubs);
|
||||
} catch (e) {
|
||||
console.error('subsUnion incremental worker failed, falling back to a full recreate', e);
|
||||
recomputePending = true;
|
||||
return;
|
||||
}
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
await storeUnion(isPublic, union, countSubs);
|
||||
}
|
||||
if (debug) console.log('Subs union updated incrementally');
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue