Skip to content

Commit 35b25a9

Browse files
committed
Image Super Cache
Performance observations from benchmarking image reads showed that we were spending a very large amount of time converting images, part of the solution to this is to actually do the conversion on read and cache that result. There is also a fairly significant heap/garbage collection penalty too. The other important observation is that on a cache refresh someone has to incur the cost. If we blocked start up to allow caches to be pre-heated, and also performed the refreshes in the background, then no one has to suffer. This change replaces the toy cache with a fully fledged refresh aside implementation. Performance metrics show a 66% time reduction in reading images, mostly due presumably to the conversion being cached, but also aided by new zero-copy semantics. The refresh aside cache also supports data epochs, so we pass the whole lot up to the APIs which can now combine that with any query related data and cache the JSON formatted data, which would save another huge amount of cycles and memory.
1 parent ff8c84a commit 35b25a9

File tree

13 files changed

+154
-153
lines changed

13 files changed

+154
-153
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ require (
1515
github.com/spf13/pflag v1.0.10
1616
github.com/spjmurray/go-util v0.1.3
1717
github.com/stretchr/testify v1.11.1
18-
github.com/unikorn-cloud/core v1.14.0-rc1
19-
github.com/unikorn-cloud/identity v1.14.0-rc1
18+
github.com/unikorn-cloud/core v1.14.0-rc1.0.20260213113949-5c30aaff8409
19+
github.com/unikorn-cloud/identity v1.14.0-rc1.0.20260209130807-b25e8c7ed9ac
2020
go.opentelemetry.io/otel v1.35.0
2121
go.opentelemetry.io/otel/trace v1.35.0
2222
go.uber.org/mock v0.5.2

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,10 @@ github.com/test-go/testify v1.1.4 h1:Tf9lntrKUMHiXQ07qBScBTSA0dhYQlu83hswqelv1iE
203203
github.com/test-go/testify v1.1.4/go.mod h1:rH7cfJo/47vWGdi4GPj16x3/t1xGOj2YxzmNQzk2ghU=
204204
github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE=
205205
github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg=
206-
github.com/unikorn-cloud/core v1.14.0-rc1 h1:5kq5v3hjXnDNPBy1BsDLSwY233u8XLYwyleywJeTFUo=
207-
github.com/unikorn-cloud/core v1.14.0-rc1/go.mod h1:NBcUrdV7RKIPFBdootNeN74otIoNLQp14+KpVE5m1X0=
208-
github.com/unikorn-cloud/identity v1.14.0-rc1 h1:B/G9Lp27qPQHnKPNrfOJlY7YnahMSlv38cwtHC/mzkY=
209-
github.com/unikorn-cloud/identity v1.14.0-rc1/go.mod h1:6mJqFyX8QZHvNFV0aYZJrE/1eozR5xiZYORNxe1Xlxc=
206+
github.com/unikorn-cloud/core v1.14.0-rc1.0.20260213113949-5c30aaff8409 h1:7yYJHVFjebQF+Nf5K3bR7QyGlVKlxqTXTfDauoGoIsI=
207+
github.com/unikorn-cloud/core v1.14.0-rc1.0.20260213113949-5c30aaff8409/go.mod h1:NBcUrdV7RKIPFBdootNeN74otIoNLQp14+KpVE5m1X0=
208+
github.com/unikorn-cloud/identity v1.14.0-rc1.0.20260209130807-b25e8c7ed9ac h1:jsby+/f0ZP6Dr7riZqjuQ3tAAuw9g6W++N3n4ERyX34=
209+
github.com/unikorn-cloud/identity v1.14.0-rc1.0.20260209130807-b25e8c7ed9ac/go.mod h1:ogNk6VL1iWYd7UOfn9jgm/Cd8Q8aXzaOhrsg+5jGrtc=
210210
github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
211211
github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
212212
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

pkg/handler/handler_v2_image_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/stretchr/testify/require"
2828
"go.uber.org/mock/gomock"
2929

30+
"github.com/unikorn-cloud/core/pkg/util/cache"
3031
"github.com/unikorn-cloud/region/pkg/handler/common"
3132
imagemock "github.com/unikorn-cloud/region/pkg/handler/image/mock"
3233
"github.com/unikorn-cloud/region/pkg/openapi"
@@ -57,7 +58,7 @@ func Test_Imagev2_List(t *testing.T) {
5758
"available to org": {
5859
setupQuery: func(query *imagemock.MockImageQuery) {
5960
query.EXPECT().AvailableToOrganization(orgID).Return(query)
60-
query.EXPECT().List(gomock.Any()).Return(nil, nil)
61+
query.EXPECT().List().Return(&cache.ListSnapshot[types.Image]{}, nil)
6162
},
6263
params: openapi.GetApiV2RegionsRegionIDImagesParams{
6364
OrganizationID: ptr.To([]string{orgID}),
@@ -67,7 +68,7 @@ func Test_Imagev2_List(t *testing.T) {
6768
setupQuery: func(query *imagemock.MockImageQuery) {
6869
query.EXPECT().AvailableToOrganization(orgID).Return(query)
6970
query.EXPECT().StatusIn(types.ImageStatusReady).Return(query)
70-
query.EXPECT().List(gomock.Any()).Return(nil, nil)
71+
query.EXPECT().List().Return(&cache.ListSnapshot[types.Image]{}, nil)
7172
},
7273
params: openapi.GetApiV2RegionsRegionIDImagesParams{
7374
OrganizationID: ptr.To([]string{orgID}),
@@ -77,7 +78,7 @@ func Test_Imagev2_List(t *testing.T) {
7778
"owned by org": {
7879
setupQuery: func(query *imagemock.MockImageQuery) {
7980
query.EXPECT().OwnedByOrganization(orgID).Return(query)
80-
query.EXPECT().List(gomock.Any()).Return(nil, nil)
81+
query.EXPECT().List().Return(&cache.ListSnapshot[types.Image]{}, nil)
8182
},
8283
params: openapi.GetApiV2RegionsRegionIDImagesParams{
8384
OrganizationID: ptr.To([]string{orgID}),
@@ -88,7 +89,7 @@ func Test_Imagev2_List(t *testing.T) {
8889
setupQuery: func(query *imagemock.MockImageQuery) {
8990
query.EXPECT().AvailableToOrganization(orgID).Return(query)
9091
query.EXPECT().StatusIn(types.ImageStatusReady).Return(query)
91-
query.EXPECT().List(gomock.Any()).Return(nil, nil)
92+
query.EXPECT().List().Return(&cache.ListSnapshot[types.Image]{}, nil)
9293
},
9394
params: openapi.GetApiV2RegionsRegionIDImagesParams{
9495
OrganizationID: ptr.To([]string{orgID}),
@@ -101,7 +102,7 @@ func Test_Imagev2_List(t *testing.T) {
101102
// This must be called even though the caller has no permission to the org,
102103
// to get global images, since those are counted as available.
103104
query.EXPECT().AvailableToOrganization().Return(query)
104-
query.EXPECT().List(gomock.Any()).Return(nil, nil)
105+
query.EXPECT().List().Return(&cache.ListSnapshot[types.Image]{}, nil)
105106
},
106107
params: openapi.GetApiV2RegionsRegionIDImagesParams{
107108
OrganizationID: ptr.To([]string{"NOT" + orgID}),
@@ -110,7 +111,7 @@ func Test_Imagev2_List(t *testing.T) {
110111
"no filters": { // when asking without giving an organization, you don't need org permissions.
111112
setupQuery: func(query *imagemock.MockImageQuery) {
112113
query.EXPECT().AvailableToOrganization().Return(query)
113-
query.EXPECT().List(gomock.Any()).Return(nil, nil)
114+
query.EXPECT().List().Return(&cache.ListSnapshot[types.Image]{}, nil)
114115
},
115116
},
116117
}

pkg/handler/image/conversions.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,11 @@ func convertImage(in *types.Image) *openapi.Image {
185185
return out
186186
}
187187

188-
func convertImages(in []types.Image) openapi.Images {
189-
out := make(openapi.Images, len(in))
188+
func convertImages(in types.ImageList) openapi.Images {
189+
out := make(openapi.Images, len(in.Items))
190190

191-
for i := range in {
192-
out[i] = *convertImage(&in[i])
191+
for i := range in.Items {
192+
out[i] = *convertImage(in.Items[i])
193193
}
194194

195195
return out

pkg/handler/image/image.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,14 @@ func (c *Client) ListImages(ctx context.Context, organizationID, regionID string
6464
result, err := query.
6565
AvailableToOrganization(organizationID).
6666
StatusIn(types.ImageStatusReady).
67-
List(ctx)
67+
List()
6868

6969
if err != nil {
7070
return nil, err
7171
}
7272

7373
// Apply ordering guarantees, ordered by name.
74-
slices.SortStableFunc(result, func(a, b types.Image) int {
74+
slices.SortStableFunc(result.Items, func(a, b *types.Image) int {
7575
return cmp.Compare(a.Name, b.Name)
7676
})
7777

pkg/handler/image/mock/queryinterfaces.go

Lines changed: 4 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/handler/image/query.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ func (c *Client) QueryImages(ctx context.Context, regionID string, params openap
5656
query = query.StatusIn(queryStatuses...)
5757
}
5858

59-
result, err := query.List(ctx)
59+
result, err := query.List()
6060
if err != nil {
6161
return nil, err
6262
}
6363

6464
// Apply ordering guarantees, ordered by name.
65-
slices.SortStableFunc(result, func(a, b types.Image) int {
65+
slices.SortStableFunc(result.Items, func(a, b *types.Image) int {
6666
return cmp.Compare(a.Name, b.Name)
6767
})
6868

pkg/providers/internal/openstack/export_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ import (
2121
"context"
2222

2323
"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers"
24-
"github.com/gophercloud/gophercloud/v2/openstack/image/v2/images"
2524
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/routers"
2625
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/groups"
2726
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports"
2827
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/subnets"
2928

29+
"github.com/unikorn-cloud/core/pkg/util/cache"
3030
unikornv1 "github.com/unikorn-cloud/region/pkg/apis/unikorn/v1alpha1"
3131
"github.com/unikorn-cloud/region/pkg/providers/types"
3232

@@ -35,7 +35,7 @@ import (
3535

3636
// NewImageQuery makes the internal implementation of ImageQuery available for testing
3737
// on its own.
38-
func NewImageQuery(listFunc func(context.Context) ([]images.Image, error)) types.ImageQuery {
38+
func NewImageQuery(listFunc func() (*cache.ListSnapshot[types.Image], error)) types.ImageQuery {
3939
return &imageQuery{listFunc: listFunc}
4040
}
4141

0 commit comments

Comments
 (0)