reduce lock time

sreya · sreya · commit 84d9556a1022 · 2024-09-24T20:17:26.000Z
diff --git a/coderd/cryptokeys/dbkeycache.go b/coderd/cryptokeys/dbkeycache.go
@@ -40,12 +40,13 @@ func NewDBKeyCache(ctx context.Context, logger slog.Logger, db database.Store, f
 		opt(d)
 	}
 
-	err := d.newCache(ctx)
+	cache, latest, err := d.newCache(ctx)
 	if err != nil {
 		return nil, xerrors.Errorf("new cache: %w", err)
 	}
+	d.cache, d.latestKey = cache, latest
 
-	go d.refreshCache(ctx)
+	go d.refresh(ctx)
 	return d, nil
 }
 
@@ -111,38 +112,44 @@ func (d *DBKeyCache) Latest(ctx context.Context) (database.CryptoKey, error) {
 		return d.latestKey, nil
 	}
 
-	err := d.newCache(ctx)
+	cache, latest, err := d.newCache(ctx)
 	if err != nil {
 		return database.CryptoKey{}, xerrors.Errorf("new cache: %w", err)
 	}
 
-	if len(d.cache) == 0 {
+	if len(cache) == 0 {
 		return database.CryptoKey{}, ErrKeyNotFound
 	}
 
-	if !d.latestKey.IsActive(now) {
+	if !latest.IsActive(now) {
 		return database.CryptoKey{}, ErrKeyInvalid
 	}
 
+	d.cache, d.latestKey = cache, latest
+
 	return d.latestKey, nil
 }
 
-func (d *DBKeyCache) refreshCache(ctx context.Context) {
+func (d *DBKeyCache) refresh(ctx context.Context) {
 	d.Clock.TickerFunc(ctx, time.Minute*10, func() error {
-		d.cacheMu.Lock()
-		defer d.cacheMu.Unlock()
-		if err := d.newCache(ctx); err != nil {
+		cache, latest, err := d.newCache(ctx)
+		if err != nil {
 			d.logger.Error(ctx, "failed to refresh cache", slog.Error(err))
+			return nil
 		}
+		d.cacheMu.Lock()
+		defer d.cacheMu.Unlock()
+
+		d.cache, d.latestKey = cache, latest
 		return nil
 	})
 }
 
-func (d *DBKeyCache) newCache(ctx context.Context) error {
+func (d *DBKeyCache) newCache(ctx context.Context) (map[int32]database.CryptoKey, database.CryptoKey, error) {
 	now := d.Clock.Now().UTC()
 	keys, err := d.db.GetCryptoKeysByFeature(ctx, d.feature)
 	if err != nil {
-		return xerrors.Errorf("get crypto keys by feature: %w", err)
+		return nil, database.CryptoKey{}, xerrors.Errorf("get crypto keys by feature: %w", err)
 	}
 	cache := toMap(keys)
 	var latest database.CryptoKey
@@ -154,9 +161,7 @@ func (d *DBKeyCache) newCache(ctx context.Context) error {
 		break
 	}
 
-	d.cache = cache
-	d.latestKey = latest
-	return nil
+	return cache, latest, nil
 }
 
 func toMap(keys []database.CryptoKey) map[int32]database.CryptoKey {

Original file line number	Diff line number	Diff line change
`@@ -40,12 +40,13 @@ func NewDBKeyCache(ctx context.Context, logger slog.Logger, db database.Store, f`
`40`	`40`	`opt(d)`
`41`	`41`	`}`
`42`	`42`
`43`		`- err := d.newCache(ctx)`
	`43`	`+ cache, latest, err := d.newCache(ctx)`
`44`	`44`	`if err != nil {`
`45`	`45`	`return nil, xerrors.Errorf("new cache: %w", err)`
`46`	`46`	`}`
	`47`	`+ d.cache, d.latestKey = cache, latest`
`47`	`48`
`48`		`- go d.refreshCache(ctx)`
	`49`	`+ go d.refresh(ctx)`
`49`	`50`	`return d, nil`
`50`	`51`	`}`
`51`	`52`
`@@ -111,38 +112,44 @@ func (d *DBKeyCache) Latest(ctx context.Context) (database.CryptoKey, error) {`
`111`	`112`	`return d.latestKey, nil`
`112`	`113`	`}`
`113`	`114`
`114`		`- err := d.newCache(ctx)`
	`115`	`+ cache, latest, err := d.newCache(ctx)`
`115`	`116`	`if err != nil {`
`116`	`117`	`return database.CryptoKey{}, xerrors.Errorf("new cache: %w", err)`
`117`	`118`	`}`
`118`	`119`
`119`		`- if len(d.cache) == 0 {`
	`120`	`+ if len(cache) == 0 {`
`120`	`121`	`return database.CryptoKey{}, ErrKeyNotFound`
`121`	`122`	`}`
`122`	`123`
`123`		`- if !d.latestKey.IsActive(now) {`
	`124`	`+ if !latest.IsActive(now) {`
`124`	`125`	`return database.CryptoKey{}, ErrKeyInvalid`
`125`	`126`	`}`
`126`	`127`
	`128`	`+ d.cache, d.latestKey = cache, latest`
	`129`	`+`
`127`	`130`	`return d.latestKey, nil`
`128`	`131`	`}`
`129`	`132`
`130`		`-func (d *DBKeyCache) refreshCache(ctx context.Context) {`
	`133`	`+func (d *DBKeyCache) refresh(ctx context.Context) {`
`131`	`134`	`d.Clock.TickerFunc(ctx, time.Minute*10, func() error {`
`132`		`- d.cacheMu.Lock()`
`133`		`- defer d.cacheMu.Unlock()`
`134`		`- if err := d.newCache(ctx); err != nil {`
	`135`	`+ cache, latest, err := d.newCache(ctx)`
	`136`	`+ if err != nil {`
`135`	`137`	`d.logger.Error(ctx, "failed to refresh cache", slog.Error(err))`
	`138`	`+ return nil`
`136`	`139`	`}`
	`140`	`+ d.cacheMu.Lock()`
	`141`	`+ defer d.cacheMu.Unlock()`
	`142`	`+`
	`143`	`+ d.cache, d.latestKey = cache, latest`
`137`	`144`	`return nil`
`138`	`145`	`})`
`139`	`146`	`}`
`140`	`147`
`141`		`-func (d *DBKeyCache) newCache(ctx context.Context) error {`
	`148`	`+func (d *DBKeyCache) newCache(ctx context.Context) (map[int32]database.CryptoKey, database.CryptoKey, error) {`
`142`	`149`	`now := d.Clock.Now().UTC()`
`143`	`150`	`keys, err := d.db.GetCryptoKeysByFeature(ctx, d.feature)`
`144`	`151`	`if err != nil {`
`145`		`- return xerrors.Errorf("get crypto keys by feature: %w", err)`
	`152`	`+ return nil, database.CryptoKey{}, xerrors.Errorf("get crypto keys by feature: %w", err)`
`146`	`153`	`}`
`147`	`154`	`cache := toMap(keys)`
`148`	`155`	`var latest database.CryptoKey`
`@@ -154,9 +161,7 @@ func (d *DBKeyCache) newCache(ctx context.Context) error {`
`154`	`161`	`break`
`155`	`162`	`}`
`156`	`163`
`157`		`- d.cache = cache`
`158`		`- d.latestKey = latest`
`159`		`- return nil`
	`164`	`+ return cache, latest, nil`
`160`	`165`	`}`
`161`	`166`
`162`	`167`	`func toMap(keys []database.CryptoKey) map[int32]database.CryptoKey {`