[fix] don't remove cache on complete, instead use scheduled cleaners#4078
[fix] don't remove cache on complete, instead use scheduled cleaners#4078zsculac merged 7 commits intov8/developfrom
Conversation
the cache cleaner now runs every 1h but frees only from memory, for disk it waits for 24h as before
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 7
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| null, | ||
| cachedData, | ||
| this.completedStatuses, | ||
| ); |
There was a problem hiding this comment.
Bug: UpdateService still removes cache on complete unlike PublishService
The PR's intent is to "don't remove cache on complete, instead use scheduled cleaners" as stated in the title. PublishService was updated to retrieve and preserve cached data when calling markOperationAsCompleted. However, UpdateService.processResponse still passes null to markOperationAsCompleted at line 79, which causes active cache removal on update completion. Since UpdateService has a nearly identical pattern to PublishService (both cache user-submitted data and replicate to the network), this appears to be an incomplete implementation of the stated fix.
src/service/operation-id-service.js
Outdated
|
|
||
| const fileList = await this.fileService.readDirectory(cacheFolderPath); | ||
| let total = 0; | ||
| for (const fileName of fileList) { |
There was a problem hiding this comment.
We can speed this up with Promise.all
| } else { | ||
| await this.operationIdService.cacheOperationIdDataToMemory(operationId, responseData); | ||
| await this.operationIdService.cacheOperationIdDataToFile(operationId, responseData); | ||
| } |
There was a problem hiding this comment.
If we are passing data we read from cache as responseData, why do we want to cache it again? Seems wasteful, perhaps we can remove this. Or split into 2 separate functions like we talked about
|
Don't forget to increase the version in package.json and package-lock.json |
cb4043f to
7850caf
Compare
the cache cleaner now runs every 1h but frees only from memory, for disk it waits for 24h as before
Note
Retains operation cache on completion while introducing hourly memory cleanup (24h for files), footprint logging, and supporting unit tests.
operation-service:markOperationAsCompletednow acceptsoptions.reuseExistingCacheto avoid rewriting file cache; updates status earlier.publish-service: on min replication, reuses existing cached data and completes with{ reuseExistingCache: true }.constants: addOPERATION_ID_MEMORY_CLEANUP_TIME_MILLS(1h); keepOPERATION_ID_COMMAND_CLEANUP_TIME_MILLS(24h) for files.operation-id-cleaner-command: run every 1h; clean memory with 1h TTL and files with 24h TTL.operation-id-cleaner-command: logs pre-clean cache footprint (memory/files) with graceful error handling.operation-id-service: addgetOperationIdMemoryCacheSizeBytes()andgetOperationIdFileCacheSizeBytes().Written by Cursor Bugbot for commit eef936b. This will update automatically on new commits. Configure here.