Skip to content

feat: add LRU bucket metadata cache to presigned-url-plugin#946

Merged
pyramation merged 1 commit intomainfrom
devin/1775023557-bucket-cache
Apr 1, 2026
Merged

feat: add LRU bucket metadata cache to presigned-url-plugin#946
pyramation merged 1 commit intomainfrom
devin/1775023557-bucket-cache

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

@pyramation pyramation commented Apr 1, 2026

Summary

Adds an LRU cache for bucket metadata in the presigned-url-plugin. Previously, every requestUploadUrl call queried the buckets table even though bucket config (type, allowed_mime_types, max_file_size, etc.) is essentially static. Now the first request per databaseId:bucketKey pair hits the DB, and subsequent requests are served from cache.

Cache details:

  • Composite key: bucket:${databaseId}:${bucketKey}
  • Same TTL as the existing storage module cache (5min dev / 1hr prod)
  • max: 500 entries (vs 50 for storage module cache, since there are many more buckets)
  • clearStorageModuleCache() now clears both caches (coordinated eviction)
  • clearBucketCache(databaseId?) added for targeted per-database eviction

Files changed:

  • storage-module-cache.ts — new bucketCache LRU, getBucketConfig(), clearBucketCache()
  • plugin.tsrequestUploadUrl now calls getBucketConfig() instead of inline SQL
  • index.ts — exports getBucketConfig and clearBucketCache

Review & Testing Checklist for Human

  • RLS bypass on cache hits: On cache miss the bucket query runs under the calling user's pgSettings (RLS-enforced). On cache hits, the cached result is returned to any user. Verify that all org members should always have identical bucket visibility (via AuthzEntityMembership) and that no bucket-level RLS restricts access to specific users within an org.
  • Stale cache on bucket mutation: If a bucket's allowed_mime_types or max_file_size is updated, or a bucket is deleted, the cache will serve stale data for up to TTL (1hr prod). Confirm this is acceptable, or whether a cache-bust hook is needed on bucket DDL/DML.
  • No test coverage: There are no unit or integration tests for the cache hit/miss behavior, TTL expiry, or clearBucketCache prefix-scan eviction. Consider whether tests should be added before merge.

Suggested manual test: Call requestUploadUrl twice with the same bucket key and verify the second call doesn't produce a Bucket cache miss debug log line.

Notes

  • The bucketsQualifiedName interpolation in the SQL query is carried over from the previous inline query — not a new pattern.
  • clearBucketCache iterates bucketCache.keys() while deleting; this is safe with lru-cache v10's iterator implementation, but worth being aware of.

Link to Devin session: https://app.devin.ai/sessions/4c882ba2dfbf4045adf85fb83cde6f77
Requested by: @pyramation


Open with Devin

- Add bucketCache LRU (max 500, same TTL as storage module cache)
- Composite key: bucket:${databaseId}:${bucketKey}
- getBucketConfig() checks cache first, falls back to DB query
- clearStorageModuleCache() now clears both caches (coordinated eviction)
- clearBucketCache() for per-database eviction
- plugin.ts requestUploadUrl uses cached bucket lookup instead of per-request SQL query
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@pyramation pyramation merged commit 490508e into main Apr 1, 2026
45 checks passed
@pyramation pyramation deleted the devin/1775023557-bucket-cache branch April 1, 2026 07:32
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

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.

1 participant