feat: don't prevent sui-indexer running on locked RedeemSolver#362
Merged
robert-zaremba merged 4 commits intomasterfrom Feb 20, 2026
Merged
feat: don't prevent sui-indexer running on locked RedeemSolver#362robert-zaremba merged 4 commits intomasterfrom
robert-zaremba merged 4 commits intomasterfrom
Conversation
Signed-off-by: Robert Zaremba <robert@zaremba.ch>
Contributor
Reviewer's GuideRefactors the Sui indexer scheduled job to acquire and manage independent locks for the indexer and redeem solver, enabling the indexer to run even when the redeem solver is locked, and updates storage locking utilities and tests to support acquiring and releasing multiple locks at once. Sequence diagram for scheduled cron running SuiIndexer and RedeemSolver with independent lockssequenceDiagram
participant Cron as CronScheduler
participant Worker as SuiIndexerWorker
participant Storage as D1Storage
participant SuiIndexer as SuiIndexer
participant RedeemSolver as RedeemSolver
Cron->>Worker: scheduled(event, env, ctx)
activate Worker
Worker->>Worker: startCronJobs(env)
Worker->>Storage: new D1Storage(env.DB)
Worker->>Storage: getActiveNetworks()
Storage-->>Worker: activeNetworks
Worker->>Worker: getSecret(env.NBTC_MINTING_SIGNER_MNEMONIC)
Worker-->>Worker: mnemonic
Worker->>Worker: createSuiClients(activeNetworks, mnemonic)
Worker-->>Worker: suiClients
Worker->>Storage: acquireLocks([lockIndexer, lockRedeemSolver], ttl)
Storage-->>Worker: [indexerLockToken, redeemSolverLockToken]
alt indexerLockToken is not null
Worker->>SuiIndexer: runSuiIndexer(storage, activeNetworks, suiClients)
else indexerLockToken is null
Worker->>Worker: logger.warn SuiIndexer lock is busy
end
alt redeemSolverLockToken is not null
Worker->>RedeemSolver: runRedeemSolver(storage, env, suiClients, activeNetworks)
else redeemSolverLockToken is null
Worker->>Worker: logger.warn RedeemSolver lock is busy
end
alt no jobs started
Worker-->>Cron: return
else at least one job started
Worker->>Worker: Promise.allSettled(jobs)
Worker-->>Worker: results
Worker->>Worker: reportErrors(results, "scheduled", "Scheduled task error", jobNames)
Worker->>Storage: releaseLocks([lockIndexer, lockRedeemSolver])
Storage-->>Worker: void
Worker-->>Cron: return
end
deactivate Worker
Class diagram for updated D1Storage locking API and worker cron entrypointclassDiagram
class D1Storage {
- db: D1Database
+ constructor(db: D1Database)
+ getActiveNetworks(): Promise~string[]~
+ acquireLocks(lockNames: string[], ttlMs: number): Promise~(number | null)[]~
+ releaseLocks(lockNames: string[]): Promise~void~
}
class SuiIndexerWorker {
+ fetch(request: Request, env: Env, ctx: ExecutionContext): Promise~Response~
+ scheduled(event: ScheduledController, env: Env, ctx: ExecutionContext): Promise~void~
+ startCronJobs(env: Env): Promise~void~
}
class SuiTasks {
+ runSuiIndexer(storage: D1Storage, activeNetworks: string[], suiClients: SuiNet[]): Promise~void~
+ runRedeemSolver(storage: D1Storage, env: Env, suiClients: SuiNet[], activeNetworks: string[]): Promise~void~
}
SuiIndexerWorker --> D1Storage : uses
SuiIndexerWorker --> SuiTasks : invokes
D1Storage ..> D1Database : wraps
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `packages/sui-indexer/src/storage.ts:885-876` </location>
<code_context>
- } catch (error) {
- logError({ msg: "Failed to release lock", method: "releaseLock", lockName }, error);
- throw error;
+ async releaseLocks(lockNames: string[]): Promise<void> {
+ for (const lockName of lockNames) {
+ try {
+ await this.db
+ .prepare(`DELETE FROM cron_locks WHERE lock_name = ?`)
+ .bind(lockName)
+ .run();
+ } catch (error) {
+ logError({ msg: "Failed to release lock", method: "releaseLock", lockName }, error);
+ throw error;
</code_context>
<issue_to_address>
**issue (bug_risk):** Releasing multiple locks stops on first error, potentially leaving later locks unreleased.
Since `releaseLocks` throws on the first failure, any subsequent lock names are never processed, which can leave some locks held even though the caller asked to release them all. Consider attempting all deletions and either aggregating/logging all errors before throwing once, or defining behavior for partial failures if that’s acceptable.
</issue_to_address>
### Comment 2
<location> `packages/sui-indexer/src/storage.ts:876-877` </location>
<code_context>
+ .bind(lockName, now, now + ttlMs)
+ .first<number>("acquired_at");
+ results.push(result ?? null);
+ } catch (error) {
+ logError({ msg: "Failed to acquire lock", method: "acquireLock", lockName }, error);
+ throw error;
+ }
</code_context>
<issue_to_address>
**nitpick:** Logged method names are now inconsistent with the new multi-lock API.
Within `acquireLocks`/`releaseLocks` you still log `method: "acquireLock"` / `"releaseLock"`, which no longer matches the multi-lock API. Please update the logged method name (or add a dedicated field for multi-lock usage) so logs clearly reflect the new behavior and remain easy to correlate when debugging.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the sui-indexer’s scheduled cron execution to use separate distributed locks for the indexer and redeem-solver jobs, so the indexer can still run even when the RedeemSolver is locked/busy.
Changes:
- Replace single-lock APIs with multi-lock
acquireLocks/releaseLocksinD1Storage. - Refactor the scheduled handler to acquire independent locks and run only the unlocked jobs in parallel.
- Update and extend lock-related unit tests to cover multi-lock acquisition behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/sui-indexer/src/storage.ts | Introduces multi-lock acquire/release helpers for cron locking. |
| packages/sui-indexer/src/index.ts | Uses two independent cron locks and runs unlocked jobs selectively. |
| packages/sui-indexer/src/storage.test.ts | Updates existing lock tests and adds new multi-lock scenarios. |
Comments suppressed due to low confidence (4)
packages/sui-indexer/src/index.ts:36
startCronJobsdoes DB reads + secret retrieval + client construction before attempting to acquire the cron locks. When locks are busy (common under overlap), this does unnecessary work and increases load/latency. Acquire the locks first (or at least check whether any lock was acquired) before fetching secrets/creating Sui clients.
async function startCronJobs(env: Env): Promise<void> {
const storage = new D1Storage(env.DB);
const activeNetworks = await storage.getActiveNetworks();
const mnemonic = await getSecret(env.NBTC_MINTING_SIGNER_MNEMONIC);
const suiClients = await createSuiClients(activeNetworks, mnemonic);
packages/sui-indexer/src/index.ts:15
PipelinePromiseis imported but not used. Since this is a Workers target, importing types from Node’sstreamis also easy to break if Node typings aren’t present. Remove this import if it’s not needed.
import type { PipelinePromise } from "stream";
packages/sui-indexer/src/storage.ts:878
- The
methodfield passed tologErrorstill saysacquireLock, but the function is nowacquireLocks. Updating this keeps logs searchable/consistent.
} catch (error) {
logError({ msg: "Failed to acquire lock", method: "acquireLock", lockName }, error);
throw error;
packages/sui-indexer/src/storage.ts:894
- The
methodfield passed tologErrorstill saysreleaseLock, but the function is nowreleaseLocks. Updating this keeps logs searchable/consistent.
} catch (error) {
logError({ msg: "Failed to release lock", method: "releaseLock", lockName }, error);
throw error;
Signed-off-by: Robert Zaremba <robert@zaremba.ch>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
packages/sui-indexer/src/index.ts:15
PipelinePromiseis imported but never used in this file. This adds noise and can confuse readers (and may trip linting in some setups). Remove the unused import.
import type { PipelinePromise } from "stream";
packages/sui-indexer/src/index.ts:36
- The cron job does expensive setup work (DB query for active networks, secret fetch, client creation) before checking whether any cron locks are available. If both locks are busy, this work is wasted every minute. Acquire the locks first and return early when none are acquired; only build
activeNetworks/suiClientsfor the jobs that will actually run.
const storage = new D1Storage(env.DB);
const activeNetworks = await storage.getActiveNetworks();
const mnemonic = await getSecret(env.NBTC_MINTING_SIGNER_MNEMONIC);
const suiClients = await createSuiClients(activeNetworks, mnemonic);
packages/sui-indexer/src/index.ts:37
- The lock names changed from the previous
cron-sui-indexerstyle toCronSuiIndexer/CronRedeemSolver. Renaming lock keys changes the distributed locking behavior during rollout (old/new deployments won’t contend on the same lock), which can allow overlapping runs. Consider keeping the existing lock key for the Sui indexer (and only introducing a new key for redeem solver), or add a migration/compat strategy.
const lockNames = ["CronSuiIndexer", "CronRedeemSolver"];
packages/sui-indexer/src/index.ts:64
- Job selection is coupled to the positional order of
lockNamesviaswitch (i). If the array order ever changes (or a new lock is inserted), it’s easy to accidentally run the wrong job under the wrong lock. Prefer mapping lock name -> job function (or iterating named tuples) to keep the lock/job association explicit.
switch (i) {
case 0:
jobs.push(runSuiIndexer(storage, activeNetworks, suiClients));
break;
case 1:
jobs.push(runRedeemSolver(storage, env, suiClients, activeNetworks));
break;
default:
logger.error({ msg: "unhandled job index: " + i });
}
packages/sui-indexer/src/storage.test.ts:669
- This test can be flaky:
nowis captured before callingacquireLocks(), andacquired_atmay equalnowin the same millisecond. UsingtoBeGreaterThan(now)can intermittently fail; usetoBeGreaterThanOrEqual(now)(or mock time) instead.
const now = Date.now();
const expiredTime = now - 10000;
await db
.prepare(
"INSERT INTO cron_locks (lock_name, acquired_at, expires_at) VALUES (?, ?, ?)",
)
.bind("lock-x", expiredTime - 60000, expiredTime)
.run();
const tokens = await storage.acquireLocks(["lock-y", "lock-x", "lock-z"], 60000);
expect(tokens).toHaveLength(3);
expect(tokens[0]).toBeGreaterThan(now);
expect(tokens[1]).toBeGreaterThan(now);
expect(tokens[2]).toBeGreaterThan(now);
sczembor
approved these changes
Feb 20, 2026
Contributor
sczembor
left a comment
sczembor
left a comment
There was a problem hiding this comment.
pre-approving, just one comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Make Sui indexer and redeem solver cron jobs independently lockable so one can run while the other is locked or busy.
Enhancements:
Tests: