sql/pgwire: avoid frequent updates to last_login_time

This patch adds a small LRU cache to avoid updating the
estimated_last_login_time overly frequently for a single user. The cache
will cause us to skip the update if the user's login time was last
updated within the past 10 minutes.

This requires us to update the tests to use a different user, since the
existing test drops `testuser` and recreates, which means that the
last_login_time will not be updated for that user as it's still in the
LRU cache.

Release note: None

Author: rafiss

pkg/sql/logictest/testdata/logic_test/user

@@ -240,9 +240,9 @@ onlyif config local-mixed-25.2
 statement ok
 DROP user testuser
 
-onlyif config local-mixed-25.2
 user testuser nodeidx=0 newsession
 
+onlyif config local-mixed-25.2
 statement error pq: password authentication failed for user testuser
 SHOW session_user
 
@@ -259,7 +259,6 @@ skipif config local-mixed-25.2
 statement ok
 DROP user testuser
 
-skipif config local-mixed-25.2
 user testuser nodeidx=0 newsession
 
 # The logictest suite currently doesn't work for external authentication methods
@@ -271,39 +270,39 @@ skipif config local-mixed-25.2
 statement error pq: user identity unknown for identity provider
 SHOW session_user
 
-subtest end
-
-subtest validate_estimated_last_login_time
-
 user root
 
 statement ok
 CREATE user IF NOT EXISTS testuser
 
-user testuser
+subtest end
+
+subtest validate_estimated_last_login_time
+
+user testuser2
 
 # Since the logictest framework does not connect with the user until a command
 # is executed, we need to run a command before checking estimated_last_login_time.
 query T
 SHOW session_user
 ----
-testuser
+testuser2
 
 user root
 
 # The estimated_last_login_time is not guaranteed to be populated synchronously,
 # so we poll until testuser's entry was updated with the last login time.
 skipif config local-mixed-25.2
 query I retry
-SELECT count(*) FROM system.users WHERE estimated_last_login_time IS NOT NULL AND username = 'testuser'
+SELECT count(*) FROM system.users WHERE estimated_last_login_time IS NOT NULL AND username = 'testuser2'
 ----
 1
 
 user root
 
 onlyif config local-mixed-25.2
 statement error pq: column "estimated_last_login_time" does not exist
-select estimated_last_login_time from [SHOW USERS] where username = 'testuser';
+select estimated_last_login_time from [SHOW USERS] where username = 'testuser2';
 
 subtest end
 

pkg/sql/pgwire/BUILD.bazel

@@ -64,6 +64,7 @@ go_library(
         "//pkg/sql/sqltelemetry",
         "//pkg/sql/types",
         "//pkg/util",
+        "//pkg/util/cache",
         "//pkg/util/ctxlog",
         "//pkg/util/duration",
         "//pkg/util/envutil",

pkg/sql/pgwire/last_login_updater.go

@@ -7,22 +7,29 @@ package pgwire
 
 import (
 	"context"
+	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql"
+	"github.com/cockroachdb/cockroach/pkg/util/cache"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil/singleflight"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 )
 
 // lastLoginUpdater handles updating the last login time for SQL users with
-// deduplication via singleflight to reduce concurrent updates.
+// deduplication via singleflight to reduce concurrent updates. It also uses
+// an LRU cache so that a user never is updated more than once every 10 minutes.
 type lastLoginUpdater struct {
 	// group ensures that there is at most one last login time update
 	// in-flight at any given time.
 	group *singleflight.Group
 	// execCfg is the executor configuration used for running update operations.
 	execCfg *sql.ExecutorConfig
+	// lastUpdateCache tracks the last time each user's login time was updated
+	// to avoid redundant database updates.
+	lastUpdateCache *cache.UnorderedCache
 
 	mu struct {
 		syncutil.Mutex
@@ -32,11 +39,39 @@ type lastLoginUpdater struct {
 	}
 }
 
+const (
+	// lastLoginCacheSize is the maximum number of users to track in the LRU
+	// cache.
+	lastLoginCacheSize = 100
+	// lastLoginEvictionTime is how long to keep entries in the cache before
+	// evicting them.
+	lastLoginEvictionTime = 10 * time.Minute
+)
+
 // newLastLoginUpdater creates a new lastLoginUpdater instance.
 func newLastLoginUpdater(execCfg *sql.ExecutorConfig) *lastLoginUpdater {
+	// Create cache with LRU eviction, limiting to lastLoginCacheSize users and entries older than lastLoginEvictionTime.
+	cacheConfig := cache.Config{
+		Policy: cache.CacheLRU,
+		ShouldEvict: func(size int, key, value interface{}) bool {
+			// Evict if we have more than lastLoginCacheSize entries or if the entry
+			// is older than lastLoginEvictionTime.
+			if size > lastLoginCacheSize {
+				return true
+			}
+			lastUpdate, ok := value.(time.Time)
+			if !ok {
+				// Evict invalid entries also.
+				return true
+			}
+			return timeutil.Since(lastUpdate) > lastLoginEvictionTime
+		},
+	}
+
 	u := &lastLoginUpdater{
-		group:   singleflight.NewGroup("update last login time", ""),
-		execCfg: execCfg,
+		group:           singleflight.NewGroup("update last login time", ""),
+		execCfg:         execCfg,
+		lastUpdateCache: cache.NewUnorderedCache(cacheConfig),
 	}
 	u.mu.pendingUsers = make(map[username.SQLUsername]struct{})
 	return u
@@ -51,6 +86,16 @@ func (u *lastLoginUpdater) updateLastLoginTime(ctx context.Context, dbUser usern
 		return
 	}
 
+	// Check if we recently updated this user's login time.
+	if lastUpdate, ok := u.lastUpdateCache.Get(dbUser); ok {
+		if lastUpdateTime, ok := lastUpdate.(time.Time); ok {
+			// Only update if it's been more than lastLoginEvictionTime since the last update.
+			if timeutil.Since(lastUpdateTime) < lastLoginEvictionTime {
+				return
+			}
+		}
+	}
+
 	// Use singleflight to ensure at most one last login time update batch
 	// is in-flight at any given time.
 	future, leader := u.group.DoChan(ctx, "UpdateLastLoginTime",
@@ -96,10 +141,21 @@ func (u *lastLoginUpdater) processPendingUpdates(ctx context.Context) error {
 	u.mu.pendingUsers = make(map[username.SQLUsername]struct{})
 	u.mu.Unlock()
 
+	if len(users) == 0 {
+		return nil
+	}
+
 	// Update last login time for all pending users in a single query.
 	err := sql.UpdateLastLoginTime(ctx, u.execCfg, users)
 	if err != nil {
 		return err
 	}
+
+	// Update the cache with the current time for all successfully updated users.
+	now := timeutil.Now()
+	for _, user := range users {
+		u.lastUpdateCache.Add(user, now)
+	}
+
 	return nil
 }