-
Notifications
You must be signed in to change notification settings - Fork 249
feat: slow plan cache #2611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
SkArchon
merged 58 commits into
main
from
milinda/eng-9010-router-investigate-customizing-our-planner-cache-to-consider
Mar 19, 2026
Merged
feat: slow plan cache #2611
Changes from 3 commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
53bb8e9
feat: expensive cache
SkArchon fa033d7
feat: expensive query cache
SkArchon 797b9a9
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon 4378c80
fix: review comments
SkArchon 4f644cf
fix: improvements
SkArchon 2515b07
fix: review comments
SkArchon 61bad8a
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon e094ee5
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon adf6498
fix: updates
SkArchon e36f2b3
fix: updates
SkArchon e53f464
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon 80c5594
fix: only persist the expensive cache elements
SkArchon ed60ea5
fix: updates
SkArchon 56f65b3
fix: updates
SkArchon 83314ce
fix: temporary skip
SkArchon 5ff028f
Merge remote-tracking branch 'origin/main' into milinda/eng-9010-rout…
SkArchon c495de1
fix: tests
SkArchon ed895d8
fix: tests
SkArchon 46c1fb5
fix: updates
SkArchon 8f2818d
fix: updates
SkArchon 731829d
fix: tests
SkArchon 4a0fed7
fix: updates
SkArchon 2d80455
fix: updates
SkArchon c708af5
fix: review comments
SkArchon 64fa885
fix: review comments
SkArchon 40dc67f
fix: review comments
SkArchon 6e0923c
fix: cleanup
SkArchon 424c999
fix: review comments
SkArchon 1c7707c
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon 478e3de
fix: review comments
SkArchon 6163132
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
StarpTech 87ac36d
fix: changes
SkArchon cce5c7f
fix: review comments
SkArchon 830dfc0
fix: refactoring
SkArchon 238c379
fix: updates
SkArchon 91c0b79
fix: comments
SkArchon 3f12de9
Merge remote-tracking branch 'origin/main' into milinda/eng-9010-rout…
SkArchon 426093f
feat: documentation
SkArchon 09a4df6
fix: updates
SkArchon 6f3cfd0
fix: updates
SkArchon 9e7540e
fix: iterators
SkArchon 8e6da6b
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon bac502b
fix: refactor
SkArchon e99824e
fix: updates
SkArchon d1ffde4
fix: review comments
SkArchon 04ee8b0
fix: review comments
SkArchon 8ded16b
fix: review comments
SkArchon a9828a0
fix: updates
SkArchon 283437c
fix: updates
SkArchon 41f64a6
fix: updates
SkArchon 4fb6860
fix: bench cleanup
SkArchon 9be8a47
fix: changes
SkArchon e393588
fix: changes
SkArchon 61cc607
Merge remote-tracking branch 'origin/main' into milinda/eng-9010-rout…
SkArchon 853135b
fix: updates
SkArchon 6cfe7cf
fix: tests
SkArchon 0148d7b
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon e0db927
Merge branch 'main' into milinda/eng-9010-router-investigate-customiz…
SkArchon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package core | ||
|
|
||
| import ( | ||
| "sync" | ||
| "time" | ||
| ) | ||
|
|
||
| // expensivePlanEntry holds a cached plan and the duration it took to plan. | ||
| type expensivePlanEntry struct { | ||
| plan *planWithMetaData | ||
| duration time.Duration | ||
| } | ||
|
|
||
| // expensivePlanCache is a bounded, mutex-protected map that holds expensive plans | ||
| // that should not be subject to TinyLFU eviction in the main cache. | ||
| type expensivePlanCache struct { | ||
| mu sync.RWMutex | ||
| entries map[uint64]*expensivePlanEntry | ||
| maxSize int | ||
| } | ||
|
|
||
| func newExpensivePlanCache(maxSize int) *expensivePlanCache { | ||
| return &expensivePlanCache{ | ||
| entries: make(map[uint64]*expensivePlanEntry, maxSize), | ||
| maxSize: maxSize, | ||
| } | ||
| } | ||
|
|
||
| func (c *expensivePlanCache) Get(key uint64) (*planWithMetaData, bool) { | ||
| c.mu.RLock() | ||
| defer c.mu.RUnlock() | ||
|
|
||
| entry, ok := c.entries[key] | ||
| if !ok { | ||
| return nil, false | ||
| } | ||
| return entry.plan, true | ||
| } | ||
|
|
||
| // Set stores a plan in the expensive cache. When at capacity, it only adds the | ||
| // new entry if its duration exceeds the current minimum; otherwise, it is skipped. | ||
| func (c *expensivePlanCache) Set(key uint64, plan *planWithMetaData, duration time.Duration) { | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| if c.entries == nil { | ||
| return | ||
| } | ||
|
|
||
| // If key already exists, update it | ||
| if _, ok := c.entries[key]; ok { | ||
| c.entries[key] = &expensivePlanEntry{plan: plan, duration: duration} | ||
| return | ||
| } | ||
|
|
||
| // If not at capacity, just add | ||
| if len(c.entries) < c.maxSize { | ||
| c.entries[key] = &expensivePlanEntry{plan: plan, duration: duration} | ||
| return | ||
| } | ||
|
|
||
| // At capacity: find the minimum and only evict if new entry is more expensive | ||
| var minKey uint64 | ||
| var minDur time.Duration | ||
| first := true | ||
| for k, e := range c.entries { | ||
| if first || e.duration < minDur { | ||
| minKey = k | ||
| minDur = e.duration | ||
| first = false | ||
| } | ||
| } | ||
|
|
||
| if duration > minDur { | ||
| delete(c.entries, minKey) | ||
| c.entries[key] = &expensivePlanEntry{plan: plan, duration: duration} | ||
| } | ||
| } | ||
|
|
||
| func (c *expensivePlanCache) IterValues(cb func(v *planWithMetaData) bool) { | ||
| c.mu.RLock() | ||
| defer c.mu.RUnlock() | ||
|
|
||
| for _, e := range c.entries { | ||
| if cb(e.plan) { | ||
| return | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (c *expensivePlanCache) Close() { | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| c.entries = nil | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,275 @@ | ||
| package core | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestExpensivePlanCache_GetSet(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
|
|
||
| plan1 := &planWithMetaData{content: "query { a }"} | ||
| plan2 := &planWithMetaData{content: "query { b }"} | ||
|
|
||
| // Miss | ||
| _, ok := c.Get(1) | ||
| require.False(t, ok) | ||
|
|
||
| // Set and get | ||
| c.Set(1, plan1, 10*time.Millisecond) | ||
| got, ok := c.Get(1) | ||
| require.True(t, ok) | ||
| require.Equal(t, plan1, got) | ||
|
|
||
| // Different key | ||
| c.Set(2, plan2, 20*time.Millisecond) | ||
| got, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| require.Equal(t, plan2, got) | ||
|
|
||
| // Original still there | ||
| got, ok = c.Get(1) | ||
| require.True(t, ok) | ||
| require.Equal(t, plan1, got) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_BoundedSize(t *testing.T) { | ||
| c := newExpensivePlanCache(3) | ||
|
|
||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Millisecond) | ||
| c.Set(3, &planWithMetaData{content: "q3"}, 30*time.Millisecond) | ||
|
|
||
| // Cache is full (3/3). Adding a 4th with higher duration should evict the shortest (key=1, 10ms) | ||
| c.Set(4, &planWithMetaData{content: "q4"}, 25*time.Millisecond) | ||
|
|
||
| // Key 1 should be evicted (it had the shortest duration: 10ms) | ||
| _, ok := c.Get(1) | ||
| require.False(t, ok, "key 1 should have been evicted") | ||
|
|
||
| // Keys 2, 3, 4 should remain | ||
| _, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| _, ok = c.Get(3) | ||
| require.True(t, ok) | ||
| _, ok = c.Get(4) | ||
| require.True(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_BoundedSize_SkipsCheaper(t *testing.T) { | ||
| c := newExpensivePlanCache(3) | ||
|
|
||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Second) | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Second) | ||
| c.Set(3, &planWithMetaData{content: "q3"}, 30*time.Second) | ||
|
|
||
| // Try to add a cheaper entry (5s < 10s minimum) — should be rejected | ||
| c.Set(4, &planWithMetaData{content: "q4"}, 5*time.Second) | ||
|
|
||
| _, ok := c.Get(4) | ||
| require.False(t, ok, "cheaper entry should not be added when cache is full") | ||
|
|
||
| // All original entries should remain | ||
| _, ok = c.Get(1) | ||
| require.True(t, ok) | ||
| _, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| _, ok = c.Get(3) | ||
| require.True(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_UpdateExisting(t *testing.T) { | ||
| c := newExpensivePlanCache(2) | ||
|
|
||
| plan1 := &planWithMetaData{content: "q1"} | ||
| plan1Updated := &planWithMetaData{content: "q1-updated"} | ||
|
|
||
| c.Set(1, plan1, 10*time.Millisecond) | ||
| c.Set(1, plan1Updated, 50*time.Millisecond) | ||
|
|
||
| got, ok := c.Get(1) | ||
| require.True(t, ok) | ||
| require.Equal(t, "q1-updated", got.content) | ||
|
|
||
| // Updating an existing key should not increase the count | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Millisecond) | ||
| _, ok = c.Get(1) | ||
| require.True(t, ok, "key 1 should still exist after adding key 2 (capacity is 2)") | ||
| _, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_IterValues(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
|
|
||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Millisecond) | ||
| c.Set(3, &planWithMetaData{content: "q3"}, 30*time.Millisecond) | ||
|
|
||
| var contents []string | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| contents = append(contents, v.content) | ||
| return false | ||
| }) | ||
| require.Len(t, contents, 3) | ||
| require.ElementsMatch(t, []string{"q1", "q2", "q3"}, contents) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_IterValues_EarlyStop(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
|
|
||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Millisecond) | ||
| c.Set(3, &planWithMetaData{content: "q3"}, 30*time.Millisecond) | ||
|
|
||
| count := 0 | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| count++ | ||
| return true // stop after first | ||
| }) | ||
| require.Equal(t, 1, count) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_Close(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
|
|
||
| c.Close() | ||
|
|
||
| // After close, entries map should be nil | ||
| _, ok := c.Get(1) | ||
| require.False(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_SetAfterClose(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
| c.Close() | ||
|
|
||
| // Set after Close should not panic | ||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
|
|
||
| _, ok := c.Get(1) | ||
| require.False(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_IterValuesEmpty(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
|
|
||
| count := 0 | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| count++ | ||
| return false | ||
| }) | ||
| require.Equal(t, 0, count) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_IterValuesAfterClose(t *testing.T) { | ||
| c := newExpensivePlanCache(10) | ||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
| c.Close() | ||
|
|
||
| count := 0 | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| count++ | ||
| return false | ||
| }) | ||
| require.Equal(t, 0, count) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_EqualDurationNotEvicted(t *testing.T) { | ||
| c := newExpensivePlanCache(2) | ||
|
|
||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Millisecond) | ||
|
|
||
| // Same duration as minimum (10ms) — should NOT evict (requires strictly greater) | ||
| c.Set(3, &planWithMetaData{content: "q3"}, 10*time.Millisecond) | ||
|
|
||
| _, ok := c.Get(3) | ||
| require.False(t, ok, "entry with equal duration should not replace minimum") | ||
| _, ok = c.Get(1) | ||
| require.True(t, ok) | ||
| _, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_MaxSizeOne(t *testing.T) { | ||
| c := newExpensivePlanCache(1) | ||
|
|
||
| c.Set(1, &planWithMetaData{content: "q1"}, 10*time.Millisecond) | ||
| got, ok := c.Get(1) | ||
| require.True(t, ok) | ||
| require.Equal(t, "q1", got.content) | ||
|
|
||
| // Adding a more expensive entry should evict the only entry | ||
| c.Set(2, &planWithMetaData{content: "q2"}, 20*time.Millisecond) | ||
| _, ok = c.Get(1) | ||
| require.False(t, ok) | ||
| got, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| require.Equal(t, "q2", got.content) | ||
|
|
||
| // Adding a cheaper entry should be rejected | ||
| c.Set(3, &planWithMetaData{content: "q3"}, 5*time.Millisecond) | ||
| _, ok = c.Get(3) | ||
| require.False(t, ok) | ||
| _, ok = c.Get(2) | ||
| require.True(t, ok) | ||
| } | ||
|
|
||
| func TestExpensivePlanCache_ConcurrentAccess(t *testing.T) { | ||
| c := newExpensivePlanCache(100) | ||
| done := make(chan struct{}) | ||
|
|
||
| // Concurrent writers — each goroutine writes to its own key range | ||
| for i := 0; i < 10; i++ { | ||
| go func(id int) { | ||
| defer func() { done <- struct{}{} }() | ||
| for j := 0; j < 100; j++ { | ||
| key := uint64(id*100 + j) | ||
| c.Set(key, &planWithMetaData{content: "q"}, time.Duration(j)*time.Millisecond) | ||
| } | ||
| }(i) | ||
| } | ||
|
|
||
| // Concurrent readers | ||
| for i := 0; i < 10; i++ { | ||
| go func(id int) { | ||
| defer func() { done <- struct{}{} }() | ||
| for j := 0; j < 100; j++ { | ||
| c.Get(uint64(id*100 + j)) | ||
| } | ||
| }(i) | ||
| } | ||
|
|
||
| // Concurrent iterators | ||
| for i := 0; i < 5; i++ { | ||
| go func() { | ||
| defer func() { done <- struct{}{} }() | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| return false | ||
| }) | ||
| }() | ||
| } | ||
|
|
||
| // Wait for all goroutines | ||
| for i := 0; i < 25; i++ { | ||
| <-done | ||
| } | ||
|
|
||
| // Cache should be at capacity and all entries should be retrievable | ||
| count := 0 | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| count++ | ||
| return false | ||
| }) | ||
| require.Equal(t, 100, count, "cache should be at max capacity") | ||
|
|
||
| // Every entry in the cache should be gettable | ||
| c.IterValues(func(v *planWithMetaData) bool { | ||
| require.Equal(t, "q", v.content) | ||
| return false | ||
| }) | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.