Skip to content

Commit

Permalink
fix: invalidate cache on failed Warmup and EngineVersion (#827)
Browse files Browse the repository at this point in the history
Invalidate the cache and stop background refreshes on failed
Warmup and EngineVersion calls.
  • Loading branch information
jackwotherspoon committed Jun 12, 2024
1 parent 0a8d60b commit c3915a6
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 3 deletions.
14 changes: 11 additions & 3 deletions dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,13 @@ func (d *Dialer) EngineVersion(ctx context.Context, icn string) (string, error)
if err != nil {
return "", err
}
i, err := d.connectionInfoCache(ctx, cn, &d.defaultDialConfig.useIAMAuthN)
c, err := d.connectionInfoCache(ctx, cn, &d.defaultDialConfig.useIAMAuthN)
if err != nil {
return "", err
}
ci, err := i.ConnectionInfo(ctx)
ci, err := c.ConnectionInfo(ctx)
if err != nil {
d.removeCached(ctx, cn, c, err)
return "", err
}
return ci.DBVersion, nil
Expand All @@ -456,7 +457,14 @@ func (d *Dialer) Warmup(ctx context.Context, icn string, opts ...DialOption) err
for _, opt := range opts {
opt(&cfg)
}
_, err = d.connectionInfoCache(ctx, cn, &cfg.useIAMAuthN)
c, err := d.connectionInfoCache(ctx, cn, &cfg.useIAMAuthN)
if err != nil {
return err
}
_, err = c.ConnectionInfo(ctx)
if err != nil {
d.removeCached(ctx, cn, c, err)
}
return err
}

Expand Down
127 changes: 127 additions & 0 deletions dialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,69 @@ func TestEngineVersionAvoidsDuplicateRefreshWithIAMAuthN(t *testing.T) {
)
}

func TestEngineVersionRemovesInvalidInstancesFromCache(t *testing.T) {
// When a dialer attempts to call EngineVersion for a
// non-existent instance, it should delete the instance from
// the cache and ensure no background refresh happens (which would be
// wasted cycles).
d, err := NewDialer(
context.Background(),
WithTokenSource(mock.EmptyTokenSource{}),
)
if err != nil {
t.Fatalf("expected NewDialer to succeed, but got error: %v", err)
}

// Populate instance map with connection info cache that will always fail
// This allows the test to verify the error case path invoking close.
badInstanceConnectionName := "doesntexist:us-central1:doesntexist"
tcs := []struct {
desc string
icn string
resp connectionInfoResp
opts []DialOption
}{
{
desc: "EngineVersion on a bad instance URI",
icn: badInstanceConnectionName,
resp: connectionInfoResp{
err: errors.New("connect info failed"),
},
},
}

for _, tc := range tcs {
t.Run(tc.desc, func(t *testing.T) {
// Manually populate the internal cache with a spy
inst, _ := instance.ParseConnName(tc.icn)
spy := &spyConnectionInfoCache{
connectInfoCalls: []connectionInfoResp{tc.resp},
}
d.cache[inst] = monitoredCache{
connectionInfoCache: spy,
}

_, err = d.EngineVersion(context.Background(), tc.icn)
if err == nil {
t.Fatal("expected EngineVersion to return error")
}
// Verify that the connection info cache was closed (to prevent
// further failed refresh operations)
if got, want := spy.closeWasCalled(), true; got != want {
t.Fatal("Close was not called")
}

// Now verify that bad connection name has been deleted from map.
d.lock.RLock()
_, ok := d.cache[inst]
d.lock.RUnlock()
if ok {
t.Fatal("connection info was not removed from cache")
}
})
}
}

func TestDialerUserAgent(t *testing.T) {
data, err := os.ReadFile("version.txt")
if err != nil {
Expand Down Expand Up @@ -517,6 +580,70 @@ func TestWarmup(t *testing.T) {
}
}

func TestWarmupRemovesInvalidInstancesFromCache(t *testing.T) {
// When a dialer attempts to Warmup for a non-existent instance,
// it should delete the instance from the cache and ensure no background
// refresh happens (which would be wasted cycles).
d, err := NewDialer(
context.Background(),
WithTokenSource(mock.EmptyTokenSource{}),
)
if err != nil {
t.Fatalf("expected NewDialer to succeed, but got error: %v", err)
}

// Populate instance map with connection info cache that will always fail
// This allows the test to verify the error case path invoking close.
badInstanceConnectionName := "doesntexist:us-central1:doesntexist"
tcs := []struct {
desc string
icn string
resp connectionInfoResp
opts []DialOption
}{
{
desc: "warmup a bad instance URI",
icn: badInstanceConnectionName,
resp: connectionInfoResp{
err: errors.New("connect info failed"),
},
opts: []DialOption{WithDialIAMAuthN(true)},
},
}

for _, tc := range tcs {
t.Run(tc.desc, func(t *testing.T) {
// Manually populate the internal cache with a spy
inst, _ := instance.ParseConnName(tc.icn)
spy := &spyConnectionInfoCache{
connectInfoCalls: []connectionInfoResp{tc.resp},
}
d.cache[inst] = monitoredCache{
connectionInfoCache: spy,
}

err = d.Warmup(context.Background(), tc.icn, tc.opts...)
if err == nil {
t.Fatal("expected Warmup to return error")
}
// Verify that the connection info cache was closed (to prevent
// further failed refresh operations)
if got, want := spy.closeWasCalled(), true; got != want {
t.Fatal("Close was not called")
}

// Now verify that bad connection name has been deleted from map.
d.lock.RLock()
_, ok := d.cache[inst]
d.lock.RUnlock()
if ok {
t.Fatal("connection info was not removed from cache")
}
})
}

}

func TestDialDialerOptsConflicts(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down

0 comments on commit c3915a6

Please sign in to comment.