Repository navigation
Conversation
commit: |
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
biggest question is around monitoring.. whether there's a benefit to having the monitor, and if so how we would structure it, given that there isn't a way to query for the suspended function (we don't have an ID available for that)
| * fast-looping worker pushes it back roughly once a minute rather than every | ||
| * iteration. | ||
| */ | ||
| export async function ensureMonitored( |
There was a problem hiding this comment.
Have we lost the monitoring entirely with this approach? Is there a way to keep it monitored somehow?
| workerMutation: v.string(), | ||
| config: vConfig.partial(), | ||
| status: vStatus, | ||
| stateId: v.id("workerState"), |
There was a problem hiding this comment.
nit: for backwards compatibility, make it deprecated - a helper that marks it as optional(any)
| workerState: defineTable({ | ||
| // Bumped each iteration & on every (re)start. A scheduled loop whose | ||
| // generation no longer matches has been superseded and exits. | ||
| generation: v.int64(), |
There was a problem hiding this comment.
Do we have any protection against multiple loops running at once?
| * resumes it. | ||
| */ | ||
| export const vStatus = v.union( | ||
| v.object({ kind: v.literal("idle") }), |
There was a problem hiding this comment.
nit: removing this will prevent pushing while the workers are idle, since it's part of the schema
|
|
||
| - **`ping`** creates the worker on first call and resumes it when it's idle. | ||
| It's a no-op while the loop is running or stopped. | ||
| - **`ping`** creates the worker on first call. It's a no-op afterward — while the |
There was a problem hiding this comment.
we should likely rename ping to ensureExists or something
| } catch (e) { | ||
| // A genuine failure in the work query or worker mutation. Suspend and | ||
| // retry after a backoff instead of dying (replaces the old monitor). | ||
| const backoffMs = | ||
| worker.config.retryBackoffMs ?? DEFAULT_CONFIG.retryBackoffMs; | ||
| console.error(`[loop] "${name}" failed — retrying in ${backoffMs}ms`, e); | ||
| console.event("retry", { name }); | ||
| requestRetry({ timeoutMs: backoffMs }); |
There was a problem hiding this comment.
this assumes we can catch the error and successfully schedule after they throw? I don't think we'll be able to schedule if the query reads too many documents, or times out. (maybe we should add transactionLimits on the runQuery / runMutation calls to at least catch that case)
| if (state.generation !== generation) { | ||
| console.debug( | ||
| `[loop] "${name}" superseded (gen ${generation} !== ${state.generation})`, | ||
| ); |
There was a problem hiding this comment.
I think we still need this
| queryArgs, | ||
| )) as BatchResult<Value>; | ||
| if (real && "batch" in real) { | ||
| console.warn(`[loop] "${name}" snapshot query mismatch`); |
There was a problem hiding this comment.
this should never happen, right? The new work would need to be in pending writes for it not to show up in snapshot query.. or if there's a future change to snapshot query where it's a different snapshot. Fine to leave it, but we shouldn't imply this is expected today
Uh oh!
There was an error while loading. Please reload this page.