refactor(miner): open worktree-allocator through openLocalStoreDb for crash-safe cleanup - #6705
Closed
ultrahighsuper wants to merge 2 commits into
Closed
Conversation
… crash-safe cleanup (JSONbored#6600) worktree-allocator.js's header comment claimed to mirror the package's local-store pattern (run-state.js / claim-ledger.js / portfolio-queue.js), but those three migrated to local-store.js's openLocalStoreDb (JSONbored#4272/JSONbored#4826) — which, beyond deduplicating the mkdirSync/chmodSync/PRAGMA busy_timeout boilerplate, registers the handle via registerCleanupResource so a SIGINT/SIGTERM/crash mid-write is flushed/closed cleanly. worktree-allocator.js still hand-rolled the open and was therefore NOT registered — despite tracking exactly the kind of leased-slot state a killed process leaves stuck. - Open via openLocalStoreDb(resolvedPath) instead of the hand-rolled mkdirSync/new DatabaseSync/chmodSync/PRAGMA sequence. - Resolve/normalize the DB path via resolveLocalStoreDbPath / normalizeLocalStoreDbPath (same env vars — LOOPOVER_MINER_WORKTREE_ALLOCATOR_DB, LOOPOVER_MINER_CONFIG_DIR, XDG_CONFIG_HOME, home default — resolve identically), matching claim-ledger.js / run-state.js. - resolveWorktreeBaseDir (the git-worktree checkout dir, not the DB path) is left untouched, per scope. - Update the header comment to state it now opens through openLocalStoreDb. Adds a test asserting opening the allocator increases cleanupResourceCount by one and its own close() decreases it back. Closes JSONbored#6600
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6705 +/- ##
==========================================
- Coverage 93.65% 93.65% -0.01%
==========================================
Files 681 682 +1
Lines 68098 68108 +10
Branches 18695 18702 +7
==========================================
+ Hits 63775 63784 +9
Misses 3347 3347
- Partials 976 977 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Contributor
Author
loopover-orb
Bot
added
the
gittensor:bug
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
1 participant
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.
What
packages/loopover-miner/lib/worktree-allocator.js's header comment claimed to mirror the package's local-store pattern (run-state.js/claim-ledger.js/portfolio-queue.js), but those three migrated tolocal-store.js'sopenLocalStoreDb(#4272/#4826) — which, beyond deduplicating themkdirSync/chmodSync/PRAGMA busy_timeoutboilerplate, registers the handle viaregisterCleanupResourceso a SIGINT/SIGTERM/crash mid-write is flushed/closed cleanly.worktree-allocator.jsstill hand-rolled its open and was therefore not registered — despite tracking exactly the kind of leased-slot state a killed process leaves stuck.Resolves #6600.
Change
openLocalStoreDb(resolvedPath)instead of the hand-rolledmkdirSync/new DatabaseSync/chmodSync/PRAGMAsequence.resolveLocalStoreDbPath/normalizeLocalStoreDbPath— the same env vars (LOOPOVER_MINER_WORKTREE_ALLOCATOR_DB,LOOPOVER_MINER_CONFIG_DIR,XDG_CONFIG_HOME, home default) resolve identically — matchingclaim-ledger.js/run-state.js.resolveWorktreeBaseDir(the git-worktree checkout directory, not the DB path) is left untouched, per scope.openLocalStoreDb.Test
Adds a case asserting opening the allocator increases
process-lifecycle.js'scleanupResourceCount()by one and its ownclose()decreases it back — the crash-safe-registration guarantee the three sibling stores already have.Locally green: the new test passes; the existing behavioral tests are unchanged. (The only failures in this file are 2 pre-existing Windows-only cases — POSIX path separators and Unix file-mode bits — unrelated to this change and green on CI's Linux.) LF-clean, eslint clean.