Skip to content

Fix version-directory cleanup race and run cloudformation-validate in a worker thread - #700

Open
satyakigh wants to merge 6 commits into
mainfrom
errors-db
Open

satyakigh wants to merge 6 commits into
mainfrom
errors-db

Conversation

@satyakigh

@satyakigh satyakigh commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Description of changes:

Database (FileDB and LMDB)

Each store keeps its data in a versioned directory (filedb/v4, lmdb/v7) and deletes older versions two minutes after startup. When 1.12.0 moved FileDB from v3 to v4, it deleted filedb/v3 while older language server processes on the same machine (other IDE windows, other IDEs) were still using it, so those processes failed on every read. That is the files.scan.error spike. Each store now writes a .heartbeat file in its own directory at startup and every 60 seconds, and cleanup skips any older directory that was used in the last 24 hours. Skipped directories are counted as oldVersion.cleanup.skipped.

cfn-validate

Every validate.error in production is the cloudformation-validate engine running out of memory on templates with many conditional (Fn::If) properties. The engine ran on the main thread, so a large template froze the language server for seconds, and once it ran out of memory every later validation failed too. The engine now runs in a worker thread with the same 128 MB heap limit as cfn-lint, and each validation has a 30 second timeout. If the worker crashes, exits, or times out, it is terminated, its memory is released, and validation is turned off for the rest of the session with a single validate.error. The main thread is never blocked, and one template is validated at a time.

Worker and retry failures now use typed error classes (WorkerExitError, WorkerFailureError, RetryError) instead of plain Error; messages are unchanged.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@satyakigh
satyakigh requested a review from a team as a code owner October 6, 2026 21:30
@satyakigh
satyakigh added this pull request to stack #701 October 7, 2026 20:03
Comment thread tst/unit/services/cfnLint/PyodideWorkerManager.test.ts Fixed
Comment thread tst/unit/services/cfnLint/PyodideWorkerManager.test.ts Fixed
@satyakigh satyakigh changed the title Fix version-directory cleanup race and add error type labels to telemetry Fix version-directory cleanup race and run cloudformation-validate in a worker thread Oct 7, 2026
@github-code-quality

github-code-quality Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/vitest

The overall line coverage in commit 1a9dde0 in the errors-db branch remains at 91%, unchanged from commit e1dc3fe in the main branch.

Show a line coverage summary of the most impacted files.
File main e1dc3fe errors-db 1a9dde0 +/-
src/telemetry/S...pedTelemetry.ts 96% 87% -9%
src/datastore/F...StoreFactory.ts 93% 93% 0%
src/datastore/L...StoreFactory.ts 90% 90% 0%
src/datastore/V...ionDirectory.ts 100% 100% 0%
src/services/cf...orkerManager.ts 87% 87% 0%
src/utils/error...ErrorClasses.ts 100% 100% 0%
src/services/cf...idateService.ts 98% 100% +2%
src/datastore/DataStore.ts 92% 94% +2%
src/services/cf...lidateEngine.ts 93% 97% +4%
src/datastore/l...b/LMDBModule.ts 0% 100% +100%

Updated October 09, 2026 22:13 UTC

mrinaudo-aws
mrinaudo-aws previously approved these changes Oct 8, 2026

@mrinaudo-aws mrinaudo-aws left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Base automatically changed from npm-publish to main October 9, 2026 21:44
@satyakigh
satyakigh dismissed mrinaudo-aws’s stale review October 9, 2026 21:44

The merge-base changed after approval.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants