Skip to content

Commit a1e0933

Browse files
fix: reduce sync worker ticker period to 5m to prevent threshold drift at minimum sync interval (#4023)
## Summary The sync worker ticker period was set to 1 hour, which created a subtle scheduling bug: when `pricingSyncInterval` is set near the minimum supported value, the few seconds a sync takes to complete causes the next ticker wake-up to land just under the elapsed-time threshold, effectively doubling the actual sync cadence. Reducing the ticker period to 5 minutes ensures the check granularity stays well below the minimum supported `pricingSyncInterval`, preventing ticker drift from defeating the threshold check. ## Changes - Reduced `syncWorkerTickerPeriod` from 1 hour to 5 minutes so that ticker drift (caused by sync execution time) does not push the next wake-up just under the `pricingSyncInterval` threshold and inadvertently double the effective sync interval. - Updated the scheduling model comment to accurately describe the relationship between the ticker period and `pricingSyncInterval`, removing the outdated note that implied the 1-hour ticker was a hard lower bound on sync frequency. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/modelcatalog/... ``` Set `pricingSyncInterval` to a value near `MinimumPricingSyncIntervalSec` and verify that syncs occur at the expected cadence without doubling. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Increased frequency of pricing synchronization checks from hourly intervals to 5-minute intervals, improving update timeliness and overall system reliability across different configurations. * **Documentation** * Updated internal documentation clarifying the pricing synchronization scheduling mechanism and how timing parameters influence sync behavior. <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/4023?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 2c0e7cf commit a1e0933

2 files changed

Lines changed: 10 additions & 12 deletions

File tree

framework/modelcatalog/config.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ const (
1111
// syncWorkerTickerPeriod is the fixed interval at which the background sync worker
1212
// wakes up to check whether a sync is due. This is independent of pricingSyncInterval —
1313
// the ticker defines the check granularity, not the sync frequency.
14-
// Setting pricingSyncInterval below this value has no effect on actual sync frequency.
15-
syncWorkerTickerPeriod = 1 * time.Hour
14+
// Kept well below MinimumPricingSyncIntervalSec so the threshold check is not
15+
// defeated by ticker drift when pricingSyncInterval is set near the minimum.
16+
syncWorkerTickerPeriod = 5 * time.Minute
1617

1718
ConfigLastPricingSyncKey = "LastModelPricingSync"
1819
ConfigLastParamsSyncKey = "LastModelParametersSync"

framework/modelcatalog/sync.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -234,19 +234,16 @@ func (mc *ModelCatalog) loadModelParametersFromDatabase(ctx context.Context) (in
234234
func (mc *ModelCatalog) startSyncWorker(ctx context.Context) {
235235
// IMPORTANT: scheduling model
236236
//
237-
// The sync worker wakes on a fixed ticker (syncWorkerTickerPeriod = 1h).
238-
// On each wake it calls checkAndSyncPricing, which checks:
237+
// The sync worker wakes on a fixed ticker (syncWorkerTickerPeriod). On each
238+
// wake it checks:
239239
//
240240
// time.Since(lastSyncTimestamp) >= pricingSyncInterval
241241
//
242-
// This means:
243-
// • pricingSyncInterval defines the *minimum elapsed time* between syncs.
244-
// • The actual sync frequency = max(syncWorkerTickerPeriod, pricingSyncInterval).
245-
// • Setting pricingSyncInterval < 1h does NOT increase sync frequency —
246-
// the hourly ticker is the hard lower bound on check granularity.
247-
//
248-
// Design rationale: avoids high-frequency polling while allowing operators to
249-
// tune how stale pricing data can get (e.g., 1h vs 24h vs 7d).
242+
// pricingSyncInterval defines the minimum elapsed time between syncs. The
243+
// ticker period is the check granularity and must stay well below the
244+
// minimum supported pricingSyncInterval, otherwise ticker drift (the few
245+
// seconds a sync takes to complete) pushes the next check just under the
246+
// threshold and the effective cadence doubles.
250247
mc.syncTicker = time.NewTicker(syncWorkerTickerPeriod)
251248
mc.wg.Add(1)
252249
go mc.syncWorker(ctx)

0 commit comments

Comments
 (0)