Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA DB-maintenance script was refactored to accept an external DB instance, now runs two sequential cleanup flows (old stats removal and dead-worker removal), adds improved error handling and logging, and introduces a main() entry that acquires and destroys the DB connection. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Script
participant DB as Database
participant Cleaner as Cleanup Routines
participant Logger as Logger
Main->>DB: getDb()
Main->>Cleaner: cleanOldStats(db)
Cleaner->>DB: query/delete PoolStats/UserStats/WorkerStats
DB-->>Cleaner: results/counts
Cleaner->>Logger: log results/errors
Main->>Cleaner: cleanDeadWorkers(db)
Cleaner->>DB: query active users, find dead workers, delete WorkerStats then Workers
DB-->>Cleaner: results/counts
Cleaner->>Logger: log results/errors
Main->>DB: db.destroy()
DB-->>Main: closed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/updateUsers.ts (1)
13-24:⚠️ Potential issue | 🟠 MajorHashrate fields should be typed as
string, notnumber, to match the API response.The
convertHashrate()function expects a string with a unit suffix (e.g.,"150.2T"), and its regex pattern requires a unit letter. TheWorkerDataandUserDatainterfaces incorrectly declarehashrate*fields asnumber. While the code calls.toString()before passing toconvertHashrate()and happens to work (because the API actually returns strings, and.toString()on a string is a no-op), the type mismatch is misleading. If someone tried to use these hashrate fields as actual numbers before the.toString()call, or if the API contract changed, it would break silently. Align the interface types with what the API actually returns (strings with unit suffixes).Affects:
WorkerData(lines 13-24) andUserData(lines 26-39)
🤖 Fix all issues with AI agents
In `@scripts/cleanOldStats.ts`:
- Line 8: The import list from 'typeorm' includes an unused symbol Not; remove
Not from the import statement so only In and LessThan are imported (update the
import in scripts/cleanOldStats.ts where "In, Not, LessThan" is declared), and
run lint to verify the unused-import warning is resolved.
- Around line 90-99: The finally block in main creates a new db by calling
getDb() again, leaking the original connection; change main to declare let db =
null outside the try, assign db = await getDb() inside try, use that same db for
cleanOldStats(db) and cleanDeadWorkers(db), and in finally check if (db) await
db.destroy(); reference the main function, getDb, cleanOldStats,
cleanDeadWorkers and ensure db is not redeclared as a new const in finally
(mirror the pattern used in updateUsers.ts).
In `@scripts/updateUsers.ts`:
- Around line 98-219: The batch transaction uses db.transaction and processes
many users in a single transaction (inside the for loop) so a single per-user
error puts the whole PostgreSQL transaction into an aborted state; change the
per-user handling to use SAVEPOINTs so one user's error doesn't abort the batch:
inside the for loop (where you currently do
userRepo/userStatsRepo/workerRepo/workerStatsRepo work) issue a unique SAVEPOINT
via manager.query('SAVEPOINT sp_<i>_<j>') before processing that user, wrap the
per-user logic, and on per-user catch call manager.query('ROLLBACK TO SAVEPOINT
sp_<i>_<j>') then perform the isActive update via
manager.getRepository(User).update({ address }, { isActive: false }) (or
manager.query) and continue; on success run manager.query('RELEASE SAVEPOINT
sp_<i>_<j>'). This keeps using db.transaction and the existing repositories
(userRepo, userStatsRepo, workerRepo, workerStatsRepo) while containing failures
to individual savepoints.
🧹 Nitpick comments (5)
scripts/cleanOldStats.ts (3)
94-95: Redundant.catch()— errors are already swallowed internally.Both
cleanOldStatsandcleanDeadWorkershave internaltry/catchblocks that log and swallow errors (they never re-throw). The.catch(console.error)on lines 94–95 will therefore never fire. If you do want errors to propagate (e.g. to abort early), remove the internal swallowing and let the caller handle it. Otherwise, just drop the.catch().
48-88: Dead worker deletion is not wrapped in a transaction — partial deletes possible.If the
WorkerStatsdelete succeeds (line 70) but the subsequentWorkerdelete (line 76) fails, you'll have workers with no stats but still present in the database, and on the next run thedeadWorkersfilter won't find any stats to clean for those. Consider wrapping both deletes in adb.transaction()for atomicity, consistent with the transactional approach used inupdateUsers.ts.Also,
INACTIVE_THRESHOLD_MSis constant — move it outside theforloop.Proposed sketch
async function cleanDeadWorkers(db) { try { + const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; // 7 days + const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); + const activeUsers = await db.getRepository(User).find({ where: { isActive: true }, relations: ['workers'], }); let deletedWorkersCount = 0; let deletedStatsCount = 0; for (const user of activeUsers) { - const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; - const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); - const deadWorkers = user.workers.filter( (worker) => worker.lastUpdate < threshold ); if (deadWorkers.length > 0) { const deadWorkerIds = deadWorkers.map((w) => w.id); - const statsResult = await db.getRepository(WorkerStats).delete({ - worker: { id: In(deadWorkerIds) }, - }); - deletedStatsCount += statsResult.affected || 0; - - const workersResult = await db.getRepository(Worker).delete({ - id: In(deadWorkerIds), - }); - deletedWorkersCount += workersResult.affected || 0; + await db.transaction(async (manager) => { + const statsResult = await manager.getRepository(WorkerStats).delete({ + worker: { id: In(deadWorkerIds) }, + }); + deletedStatsCount += statsResult.affected || 0; + + const workersResult = await manager.getRepository(Worker).delete({ + id: In(deadWorkerIds), + }); + deletedWorkersCount += workersResult.affected || 0; + }); } }
10-10: Missing type annotation fordbparameter in both functions.
cleanOldStats(db)andcleanDeadWorkers(db)lack a TypeScript type fordb. Consider typing it (e.g.,DataSourcefrom TypeORM) to get compile-time safety.Also applies to: 48-48
scripts/updateUsers.ts (2)
82-91: Loose equality and filesystem fallback concern.Line 83 uses
==instead of===. While it works here since both sides are strings, strict equality is idiomatic TypeScript.The
fs.readFileSyncfallback (line 85) is synchronous and blocks the event loop. For a batch maintenance script this is likely acceptable, but worth noting. More importantly, this path reads a local file usingapiUrlwhich includes the user-controlledaddress— the regex on line 70 restricts to alphanumeric, so path traversal is mitigated. Good defensive check there.Minor fix for strict equality
- if (error.cause?.code == 'ERR_INVALID_URL') { + if (error.cause?.code === 'ERR_INVALID_URL') {
56-58: Early return after "no active users" means the script still falls through to batch processing with an empty array.When
users.length === 0, the code logs but doesn'treturn. Theforloop simply won't execute, so this isn't a bug — but an explicitreturnwould clarify intent.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/cleanOldStats.ts`:
- Around line 54-58: The JSDoc above the function is inaccurate; update the
comment for cleanDeadWorkers to describe that it removes stale/dead worker
records (e.g., workers with no heartbeat or lastSeen older than threshold) from
the database rather than "old statistical records." Mention the parameters and
return behavior correctly (that it accepts a TypeORM DataSource or Connection
and deletes dead worker entries) and include any relevant details like the
criteria used to identify dead workers or the side effects (deletion) so the
docstring accurately reflects the cleanDeadWorkers function.
- Line 113: The file ends with a bare invocation "main()" missing a terminating
semicolon; add the semicolon to the invocation of the main function (replace the
trailing "main()" with "main();" so the call to main() is properly terminated).
🧹 Nitpick comments (1)
scripts/cleanOldStats.ts (1)
71-77: Hoist constant out of the loop; fix formatting on line 76.
INACTIVE_THRESHOLD_MSandthresholdare invariant across iterations — declare them before theforloop. Also, line 76 has a missing space before<(and appears to use a tab for indentation).Proposed fix
+ const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; // 7 days + const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); + for (const user of activeUsers) { - const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; // 7 days - const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); - - const deadWorkers = user.workers.filter(worker => - worker.lastUpdate< threshold + const deadWorkers = user.workers.filter( + (worker) => worker.lastUpdate < threshold );
…into remove-dead-workers
…into remove-dead-workers
…into remove-dead-workers
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/cleanOldStats.ts (1)
20-21:⚠️ Potential issue | 🟡 MinorUnused variable
fiveDaysAgo.
fiveDaysAgois declared but never referenced in any of the delete operations. Remove it to avoid confusion.Proposed fix
- const fiveDaysAgo = new Date(); - fiveDaysAgo.setDate(fiveDaysAgo.getDate() - 5); -
🧹 Nitpick comments (2)
scripts/cleanOldStats.ts (2)
71-77: Hoist constant out of loop; fix formatting on line 76.
INACTIVE_THRESHOLD_MSandthresholdare loop-invariant — move them before theforloop. Also, line 76 has a tab character and a missing space before<(flagged by prettier).Proposed fix
+ const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; // 7 days + const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); + for (const user of activeUsers) { - const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; // 7 days - const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); - - const deadWorkers = user.workers.filter(worker => - worker.lastUpdate< threshold + const deadWorkers = user.workers.filter( + (worker) => worker.lastUpdate < threshold );
49-51: Errors are swallowed — script exits with code 0 even on failure.Both
cleanOldStatsandcleanDeadWorkerscatch and log errors but never re-throw. If this runs as a cron job, a failing cleanup will silently stop working and tables will grow unbounded with no alert. Consider re-throwing after logging so the process exits with a non-zero code on failure.Example: re-throw after logging
} catch (error) { console.error('Error cleaning old stats:', error); + throw error; }(Same pattern in
cleanDeadWorkers.)Also applies to: 97-99
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@scripts/cleanOldStats.ts`:
- Around line 106-107: The two top-level await statements calling
cleanOldStats(db) and cleanDeadWorkers(db) are missing semicolons; add trailing
semicolons to the await expressions (the lines invoking cleanOldStats and
cleanDeadWorkers) so they end with ; to satisfy ESLint/Prettier and avoid ASI
pitfalls.
- Around line 102-111: The cleanup functions cleanOldStats and cleanDeadWorkers
currently swallow errors, so update handling so failures cause a non-zero exit:
either modify cleanOldStats and cleanDeadWorkers to rethrow after logging their
errors (so callers see the exception) or change main to catch any errors from
those calls and set process.exitCode = 1 (or rethrow) before calling
db.destroy(); ensure you reference the async main function and the cleanOldStats
/ cleanDeadWorkers functions so the error is not silently ignored and the
process exits with failure when cleanup fails.
- Around line 60-63: The query using db.getRepository(User).find({ where: {
isActive: true }, relations: ['workers'] }) intentionally limits cleaning to
active users' dead workers but lacks an inline explanation; add a brief comment
above that call (near the activeUsers variable) that states the rationale (e.g.,
we preserve historical worker data for inactive users or skip them for
performance/retention policy), or if the intent was to clean all users instead,
remove the isActive: true filter and update the log accordingly; reference the
activeUsers variable, the User entity, and the relations: ['workers'] usage when
making the change.
🧹 Nitpick comments (3)
scripts/cleanOldStats.ts (3)
7-8: Fix import ordering flagged by ESLint.ESLint reports that the
typeormimport should occur before the../lib/dbimport, and there should be an empty line between import groups. Consider runningeslint --fixto auto-correct these.
16-16: Add a TypeScript type annotation for thedbparameter.Both
cleanOldStatsandcleanDeadWorkersdocument the type in JSDoc (DataSource | Connection) but don't enforce it in the signature. Addingdb: DataSourcegives compile-time safety and better IDE support.-async function cleanOldStats(db) { +async function cleanOldStats(db: DataSource) {(Same applies to
cleanDeadWorkerson line 57.)
70-88: Consider a single bulk delete instead of per-user iteration.The current approach iterates over each active user, issuing up to 2 DB deletes per user with dead workers. For a maintenance script this works, but it can be simplified into a single query that finds all workers with
lastUpdate < thresholdand deletes them in bulk. This reduces round-trips from O(N) to O(1).Sketch of a bulk approach
// Find all dead workers in one query const deadWorkers = await db.getRepository(Worker).find({ where: { lastUpdate: LessThan(threshold), user: { isActive: true } }, select: ['id'], }); if (deadWorkers.length > 0) { const deadWorkerIds = deadWorkers.map(w => w.id); const statsResult = await db.getRepository(WorkerStats).delete({ worker: { id: In(deadWorkerIds) }, }); const workersResult = await db.getRepository(Worker).delete({ id: In(deadWorkerIds), }); // ...log counts }
| async function main() { | ||
| const db = await getDb(); | ||
|
|
||
| try { | ||
| await cleanOldStats(db) | ||
| await cleanDeadWorkers(db) | ||
| } finally { | ||
| const db = await getDb(); | ||
| await db.destroy(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Silent failures: errors are swallowed so the script always exits successfully.
Both cleanOldStats and cleanDeadWorkers catch and log their errors internally without rethrowing. This means main() always completes without error, and the process exits with code 0 regardless of whether cleanup actually succeeded. For a scheduled maintenance job, this makes it impossible to detect failures via process exit code or orchestrator alerting.
Consider either rethrowing after logging, or tracking a failure flag and setting process.exitCode = 1 at the end.
Option: rethrow from cleanup functions, handle in main
} catch (error) {
console.error('Error cleaning old stats:', error);
+ throw error;
} } catch (error) {
console.error('Error cleaning dead workers: ', error);
+ throw error;
} try {
- await cleanOldStats(db)
- await cleanDeadWorkers(db)
+ await cleanOldStats(db);
+ await cleanDeadWorkers(db);
+ } catch (error) {
+ process.exitCode = 1;
} finally {
await db.destroy();
}🧰 Tools
🪛 ESLint
[error] 106-106: Insert ;
(prettier/prettier)
[error] 107-107: Insert ;
(prettier/prettier)
🤖 Prompt for AI Agents
In `@scripts/cleanOldStats.ts` around lines 102 - 111, The cleanup functions
cleanOldStats and cleanDeadWorkers currently swallow errors, so update handling
so failures cause a non-zero exit: either modify cleanOldStats and
cleanDeadWorkers to rethrow after logging their errors (so callers see the
exception) or change main to catch any errors from those calls and set
process.exitCode = 1 (or rethrow) before calling db.destroy(); ensure you
reference the async main function and the cleanOldStats / cleanDeadWorkers
functions so the error is not silently ignored and the process exits with
failure when cleanup fails.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/cleanOldStats.ts (3)
1-10: Import ordering and formatting issues flagged by ESLint/Prettier.Static analysis flags:
typeormimport should come before local imports, and there should be an empty line between import groups. Line 10 has an extra blank line.Proposed fix
import 'dotenv/config'; + +import { In, LessThan } from 'typeorm'; + import { getDb } from '../lib/db'; import { PoolStats } from '../lib/entities/PoolStats'; import { User } from '../lib/entities/User'; import { UserStats } from '../lib/entities/UserStats'; import { Worker } from '../lib/entities/Worker'; import { WorkerStats } from '../lib/entities/WorkerStats'; -import { In, LessThan } from 'typeorm'; -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/cleanOldStats.ts` around lines 1 - 10, Reorder and reformat the import block so external package imports come first, then a blank line, then local project imports; specifically move the "import { In, LessThan } from 'typeorm';" line above the local imports (getDb, PoolStats, User, UserStats, Worker, WorkerStats) and ensure there is exactly one empty line separating the external imports group and the local imports group, removing the extra blank line at the end of the block.
16-16: Add TypeScript type annotation for thedbparameter.This is a
.tsfile butdbhas no type annotation. The JSDoc saysDataSource | Connection— this should be reflected in the actual signature. Same applies tocleanDeadWorkerson line 57.Proposed fix
+import { DataSource } from 'typeorm'; ... -async function cleanOldStats(db) { +async function cleanOldStats(db: DataSource) {-async function cleanDeadWorkers(db) { +async function cleanDeadWorkers(db: DataSource) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/cleanOldStats.ts` at line 16, The functions cleanOldStats and cleanDeadWorkers are missing TypeScript parameter types; update their signatures to declare the db parameter as DataSource | Connection (e.g., async function cleanOldStats(db: DataSource | Connection) and async function cleanDeadWorkers(db: DataSource | Connection)), and add the corresponding import for DataSource and Connection at the top of the file so the types resolve. Ensure any usages inside these functions still accept the typed db without further changes.
57-89: Consider replacing the N+1 pattern with bulk DB queries.This loads all active users with all their workers into memory, filters in JS, then issues 2 DELETE queries per user with dead workers. For a pool with many users/workers, this is unnecessarily heavy on both memory and round-trips.
A direct query approach would be simpler and more efficient:
Proposed refactor
async function cleanDeadWorkers(db: DataSource) { try { const INACTIVE_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; const threshold = new Date(Date.now() - INACTIVE_THRESHOLD_MS); - const activeUsers = await db.getRepository(User).find({ - where: { isActive: true }, - relations: ['workers'], - }); - - let deletedWorkersCount = 0; - let deletedStatsCount = 0; - - for (const user of activeUsers) { - const deadWorkers = user.workers.filter(worker => - worker.lastUpdate < threshold - ); - - if (deadWorkers.length > 0) { - const deadWorkerIds = deadWorkers.map(w => w.id); - const statsResult = await db.getRepository(WorkerStats).delete({ - worker: { id: In(deadWorkerIds) }, - }); - deletedStatsCount += statsResult.affected || 0; - - const workersResult = await db.getRepository(Worker).delete({ - id: In(deadWorkerIds), - }); - deletedWorkersCount += workersResult.affected || 0; - } - } + // Find all dead workers directly (across active users) + const deadWorkers = await db.getRepository(Worker).find({ + where: { + lastUpdate: LessThan(threshold), + user: { isActive: true }, + }, + select: ['id'], + }); + + if (deadWorkers.length === 0) { + console.log('No dead workers found'); + return; + } + + const deadWorkerIds = deadWorkers.map((w) => w.id); + + // Delete associated WorkerStats first + const statsResult = await db.getRepository(WorkerStats).delete({ + worker: { id: In(deadWorkerIds) }, + }); + + // Then delete the workers + const workersResult = await db.getRepository(Worker).delete({ + id: In(deadWorkerIds), + }); - console.log(`Deleted ${deletedWorkersCount} dead workers and ${deletedStatsCount} associated stats for active users`); + console.log( + `Deleted ${workersResult.affected || 0} dead workers and ${statsResult.affected || 0} associated stats` + ); console.log('Dead workers cleanup completed successfully'); } catch (error) { console.error('Error cleaning dead workers: ', error); } }This eliminates the eager-load of all workers and the per-user loop, replacing them with three targeted queries (find, delete stats, delete workers).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/cleanOldStats.ts` around lines 57 - 89, cleanDeadWorkers currently eager-loads all active Users with their workers and loops per-user, causing N+1 memory and round-trip issues; replace that logic with three bulk DB operations: (1) compute threshold as now - INACTIVE_THRESHOLD_MS, then query Worker repository to get all dead worker ids where lastUpdate < threshold and their User.isActive = true (use a join/filter instead of loading User relations), (2) delete WorkerStats in one call using WorkerStats repository with worker id IN (deadIds) and increment deletedStatsCount from the result.affected, and (3) delete Workers in one call using Worker repository with id IN (deadIds) and increment deletedWorkersCount from that result. Keep the function name cleanDeadWorkers and use User, Worker, WorkerStats, In, deletedWorkersCount and deletedStatsCount to locate and update the variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/cleanOldStats.ts`:
- Around line 102-114: The top-level async main currently calls getDb() without
handling failures and the returned Promise is unhandled; wrap invocation with a
rejection handler and fix formatting: either wrap the call in an IIFE try/catch
around getDb()/cleanOldStats/cleanDeadWorkers and ensure db.destroy() runs in
finally, or more simply append .catch(err => { processLogger.error(...) ;
process.exit(1); }) to main() so any rejection from getDb() is caught, and add
the missing semicolon after the main() call; reference the main function and the
getDb, cleanOldStats, cleanDeadWorkers, and db.destroy usage when applying the
change.
---
Duplicate comments:
In `@scripts/cleanOldStats.ts`:
- Around line 46-48: Both cleanOldStats and cleanDeadWorkers currently catch and
only console.error the exception (silently returning success), so failures never
surface; modify each function (cleanOldStats and cleanDeadWorkers) to rethrow
the caught error after logging so callers can detect failure, and update main()
to catch any error from invoked functions and set a non-zero exit code (e.g.,
process.exitCode = 1 or process.exit(1)) after logging the fatal error so the
process exits with failure for schedulers/monitoring.
- Around line 60-63: The current query uses db.getRepository(User).find({ where:
{ isActive: true }, relations: ['workers'] }) which restricts cleanup to workers
of active users only; clarify intent by either adding a short comment above this
line explaining that dead workers for inactive users are intentionally preserved
for historical/audit reasons, or change the logic to include all users (remove
the where: { isActive: true } filter) or better yet query the Worker repository
directly (e.g., Worker repository with a status/isAlive filter) so dead workers
are cleaned regardless of their parent User; update the code around the
activeUsers variable accordingly to reflect the chosen approach.
---
Nitpick comments:
In `@scripts/cleanOldStats.ts`:
- Around line 1-10: Reorder and reformat the import block so external package
imports come first, then a blank line, then local project imports; specifically
move the "import { In, LessThan } from 'typeorm';" line above the local imports
(getDb, PoolStats, User, UserStats, Worker, WorkerStats) and ensure there is
exactly one empty line separating the external imports group and the local
imports group, removing the extra blank line at the end of the block.
- Line 16: The functions cleanOldStats and cleanDeadWorkers are missing
TypeScript parameter types; update their signatures to declare the db parameter
as DataSource | Connection (e.g., async function cleanOldStats(db: DataSource |
Connection) and async function cleanDeadWorkers(db: DataSource | Connection)),
and add the corresponding import for DataSource and Connection at the top of the
file so the types resolve. Ensure any usages inside these functions still accept
the typed db without further changes.
- Around line 57-89: cleanDeadWorkers currently eager-loads all active Users
with their workers and loops per-user, causing N+1 memory and round-trip issues;
replace that logic with three bulk DB operations: (1) compute threshold as now -
INACTIVE_THRESHOLD_MS, then query Worker repository to get all dead worker ids
where lastUpdate < threshold and their User.isActive = true (use a join/filter
instead of loading User relations), (2) delete WorkerStats in one call using
WorkerStats repository with worker id IN (deadIds) and increment
deletedStatsCount from the result.affected, and (3) delete Workers in one call
using Worker repository with id IN (deadIds) and increment deletedWorkersCount
from that result. Keep the function name cleanDeadWorkers and use User, Worker,
WorkerStats, In, deletedWorkersCount and deletedStatsCount to locate and update
the variables.
| async function main() { | ||
| const db = await getDb(); | ||
|
|
||
| try { | ||
| await cleanOldStats(db); | ||
| await cleanDeadWorkers(db); | ||
| } finally { | ||
| const db = await getDb(); | ||
| await db.destroy(); | ||
| } | ||
| } | ||
|
|
||
| cleanOldStats().catch(console.error); No newline at end of file | ||
| main() | ||
|
|
There was a problem hiding this comment.
Unhandled rejection if getDb() fails, and missing semicolon.
If getDb() on line 103 throws, the rejection from main() is unhandled since line 113 has no .catch(). Also, ESLint/Prettier flags the missing semicolon on line 113.
Proposed fix
-main()
-
+main().catch((error) => {
+ console.error('Fatal error:', error);
+ process.exitCode = 1;
+});🧰 Tools
🪛 ESLint
[error] 113-114: Replace ⏎ with ;
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/cleanOldStats.ts` around lines 102 - 114, The top-level async main
currently calls getDb() without handling failures and the returned Promise is
unhandled; wrap invocation with a rejection handler and fix formatting: either
wrap the call in an IIFE try/catch around getDb()/cleanOldStats/cleanDeadWorkers
and ensure db.destroy() runs in finally, or more simply append .catch(err => {
processLogger.error(...) ; process.exit(1); }) to main() so any rejection from
getDb() is caught, and add the missing semicolon after the main() call;
reference the main function and the getDb, cleanOldStats, cleanDeadWorkers, and
db.destroy usage when applying the change.
1 participant
I have updated cleanOldStats.ts to remove workers and their related stats after 7 days without updates (the same policy used by ckpool-solo).
This addresses issue #32.
Summary by CodeRabbit