feat(metaschema): refresh the schema cache on a timer and make it concurrency-safe - #1878
feat(metaschema): refresh the schema cache on a timer and make it concurrency-safe#1878rohilsurana wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesMetaschema cache lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to A repository failure during startup can leave the service with an empty schema cache, allowing metadata validation to be skipped until a later refresh succeeds. Merge should wait for the initial load error to be handled so the service does not serve requests in this unsafe state. Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
Coverage Report for CI Build 31684086039Coverage increased (+0.1%) to 48.379%Details
Uncovered Changes
Coverage Regressions3 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 59e3f98a-7150-490f-a857-75e9217738c5
📒 Files selected for processing (8)
cmd/migrate.gocmd/serve.goconfig/sample.config.yamlcore/metaschema/config.gocore/metaschema/service.gocore/metaschema/service_test.godocs/content/docs/reference/configurations.mdxpkg/server/config.go
| func (s *Service) Init(ctx context.Context) error { | ||
| s.reload(ctx) | ||
| s.mu.RLock() | ||
| count := len(s.metaSchemaCache) | ||
| s.mu.RUnlock() | ||
| s.logger.InfoContext(ctx, "metaschemas loaded", "count", count) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return the initial cache-load error.
reload logs a Repository.List failure but discards it. Init then returns nil, so cmd/serve.go Line 167 starts the server with an empty cache. Validate returns nil for every absent schema until a later refresh succeeds.
Return the first reload error from Init. Stop startup, or wait for a successful initial load. Keep the current retain-and-log behavior for later scheduled reload failures.
Proposed direction
func (s *Service) Init(ctx context.Context) error {
- s.reload(ctx)
+ if err := s.reload(ctx); err != nil {
+ return err
+ }
s.mu.RLock()2 participants
Summary
Metaschemas hold the JSON schemas that validate entity metadata for users, organizations, groups, and roles. The metaschema service keeps every schema in an in-memory map on the service. Today that map is primed once at boot and after that it only changes on the pod that handled a write. In a multi-pod deployment a schema change made through the API reaches one pod but not the others, so those pods keep validating against the old schema until they restart. The map also has no lock, so concurrent request goroutines on one pod can race on it, which Go turns into a "concurrent map read and map write" panic.
This PR makes the cache safe for concurrent access and reloads it on a timer, so every pod picks up a schema change within a small bounded window.
Changes
sync.RWMutexand switch every service method to a pointer receiver.robfig/cronpattern as the billing sync jobs.Initto prime the cache and start the job, andCloseto stop it.app.metaschema.refresh_intervalconfig. It defaults to1m, and0disables the job for single-pod, local, and test runs.InitandCloseinto server start and shutdown, and document the new setting.Technical Details
Get,List,Validate) take a read lock. Writes (Create,Update,Delete, and the reload) take a write lock.@everyschedules from pod start, so pods that start together reload in lockstep. For this tiny table that query cost is small. Jitter or PostgresLISTEN/NOTIFYare noted as future options if it ever matters.MetaSchemaServiceinterface and its generated mock stay valid.Test Plan
go build ./...,go vet)core/metaschemapass undergo test -race, covering concurrent access alongside a reload, reload picking up a new schema, cache kept on a list error, and the refresh-disabled path