refactor: rename Lock/Unlock on KV stores to RLock/RUnlock (#22357)

authorizer/backup.go

@@ -49,10 +49,10 @@ func (b BackupService) BackupShard(ctx context.Context, w io.Writer, shardID uin
 // are intended to be used for coordinating the locking and unlocking of the kv and sql metadata
 // databases during a backup. They are made available here to allow the calls to pass-through to the
 // underlying service.
-func (b BackupService) LockKVStore() {
+func (b BackupService) RLockKVStore() {
-	b.s.LockKVStore()
+	b.s.RLockKVStore()
 }
 
-func (b BackupService) UnlockKVStore() {
+func (b BackupService) RUnlockKVStore() {
-	b.s.UnlockKVStore()
+	b.s.RUnlockKVStore()
 }

authorizer/sql_backup_restore.go

@@ -48,10 +48,10 @@ func (s SqlBackupRestoreService) RestoreSqlStore(ctx context.Context, r io.Reade
 // are intended to be used for coordinating the locking and unlocking of the kv and sql metadata
 // databases during a backup. They are made available here to allow the calls to pass-through to the
 // underlying service.
-func (s SqlBackupRestoreService) LockSqlStore() {
+func (s SqlBackupRestoreService) RLockSqlStore() {
-	s.s.LockSqlStore()
+	s.s.RLockSqlStore()
 }
 
-func (s SqlBackupRestoreService) UnlockSqlStore() {
+func (s SqlBackupRestoreService) RUnlockSqlStore() {
-	s.s.UnlockSqlStore()
+	s.s.RUnlockSqlStore()
 }

backup.go

@@ -20,11 +20,11 @@ type BackupService interface {
 	// BackupShard downloads a backup file for a single shard.
 	BackupShard(ctx context.Context, w io.Writer, shardID uint64, since time.Time) error
 
-	// LockKVStore locks the database.
+	// RLockKVStore locks the database.
-	LockKVStore()
+	RLockKVStore()
 
-	// UnlockKVStore unlocks the database.
+	// RUnlockKVStore unlocks the database.
-	UnlockKVStore()
+	RUnlockKVStore()
 }
 
 // SqlBackupRestoreService represents the backup and restore functions for the sqlite database.
@@ -35,11 +35,11 @@ type SqlBackupRestoreService interface {
 	// RestoreSqlStore restores & replaces the sqlite database.
 	RestoreSqlStore(ctx context.Context, r io.Reader) error
 
-	// LockSqlStore locks the database.
+	// RLockSqlStore takes a read lock on the database
-	LockSqlStore()
+	RLockSqlStore()
 
-	// UnlockSqlStore unlocks the database.
+	// RUnlockSqlStore releases a previously-taken read lock on the database.
-	UnlockSqlStore()
+	RUnlockSqlStore()
 }
 
 type BucketManifestWriter interface {

bolt/kv.go

@@ -107,13 +107,11 @@ func (s *KVStore) Close() error {
 	return nil
 }
 
-// LockKVStore locks the database for reading during a backup.
+func (s *KVStore) RLock() {
-func (s *KVStore) Lock() {
 	s.mu.RLock()
 }
 
-// UnlockKVStore removes the read lock used during a backup.
+func (s *KVStore) RUnlock() {
-func (s *KVStore) Unlock() {
 	s.mu.RUnlock()
 }
 

cmd/influxd/launcher/engine.go

@@ -164,12 +164,12 @@ func (t *TemporaryEngine) BackupKVStore(ctx context.Context, w io.Writer) error
 	return t.engine.BackupKVStore(ctx, w)
 }
 
-func (t *TemporaryEngine) LockKVStore() {
+func (t *TemporaryEngine) RLockKVStore() {
-	t.engine.LockKVStore()
+	t.engine.RLockKVStore()
 }
 
-func (t *TemporaryEngine) UnlockKVStore() {
+func (t *TemporaryEngine) RUnlockKVStore() {
-	t.engine.UnlockKVStore()
+	t.engine.RUnlockKVStore()
 }
 
 func (t *TemporaryEngine) RestoreKVStore(ctx context.Context, r io.Reader) error {

http/backup_service.go

@@ -140,11 +140,11 @@ func (h *BackupHandler) handleBackupMetadata(w http.ResponseWriter, r *http.Requ
 
 	// Lock the sqlite and bolt databases prior to writing the response to prevent
 	// data inconsistencies.
-	h.BackupService.LockKVStore()
+	h.BackupService.RLockKVStore()
-	defer h.BackupService.UnlockKVStore()
+	defer h.BackupService.RUnlockKVStore()
 
-	h.SqlBackupRestoreService.LockSqlStore()
+	h.SqlBackupRestoreService.RLockSqlStore()
-	defer h.SqlBackupRestoreService.UnlockSqlStore()
+	defer h.SqlBackupRestoreService.RUnlockSqlStore()
 
 	dataWriter := multipart.NewWriter(w)
 	w.Header().Set("Content-Type", "multipart/mixed; boundary="+dataWriter.Boundary())

http/backup_service_test.go

@@ -38,21 +38,15 @@ func TestBackupMetaService(t *testing.T) {
 		BackupKVStore(gomock.Any(), gomock.Any()).
 		Return(nil)
 
-	backupSvc.EXPECT().
+	backupSvc.EXPECT().RLockKVStore()
-		LockKVStore()
+	backupSvc.EXPECT().UnlockKVStore()
-
-	backupSvc.EXPECT().
-		UnlockKVStore()
 
 	sqlBackupSvc.EXPECT().
 		BackupSqlStore(gomock.Any(), gomock.Any()).
 		Return(nil)
 
-	sqlBackupSvc.EXPECT().
+	sqlBackupSvc.EXPECT().RLockSqlStore()
-		LockSqlStore()
+	sqlBackupSvc.EXPECT().RUnlockSqlStore()
-
-	sqlBackupSvc.EXPECT().
-		UnlockSqlStore()
 
 	bucketManifestWriter.EXPECT().
 		WriteManifest(gomock.Any(), gomock.Any()).

inmem/kv.go

@@ -84,13 +84,12 @@ func (s *KVStore) DeleteBucket(ctx context.Context, name []byte) error {
 	return nil
 }
 
-// Lock and unlock are only used when doing a backup, so panic if they are called here.
+func (s *KVStore) RLock() {
-func (s *KVStore) Lock() {
+	s.mu.RLock()
-	panic("not implemented")
 }
 
-func (s *KVStore) Unlock() {
+func (s *KVStore) RUnlock() {
-	panic("not implemented")
+	s.mu.RUnlock()
 }
 
 func (s *KVStore) Backup(ctx context.Context, w io.Writer) error {

kv/store.go

@@ -53,10 +53,10 @@ type Store interface {
 	Backup(ctx context.Context, w io.Writer) error
 	// Restore replaces the underlying data file with the data from r.
 	Restore(ctx context.Context, r io.Reader) error
-	// Lock locks the underlying KV store for writes
+	// RLock takes a read lock on the underlying KV store.
-	Lock()
+	RLock()
-	// Unlock unlocks the underlying KV store for writes
+	// RUnlock releases a previously-taken read lock
-	Unlock()
+	RUnlock()
 }
 
 // Tx is a transaction in the store.

mock/backup_service.go

@@ -66,27 +66,27 @@ func (mr *MockBackupServiceMockRecorder) BackupShard(ctx, w, shardID, since inte
 }
 
 // LockKVStore mocks base method.
-func (m *MockBackupService) LockKVStore() {
+func (m *MockBackupService) RLockKVStore() {
 	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "LockKVStore")
+	m.ctrl.Call(m, "RLockKVStore")
 }
 
 // LockKVStore indicates an expected call of LockKVStore.
-func (mr *MockBackupServiceMockRecorder) LockKVStore() *gomock.Call {
+func (mr *MockBackupServiceMockRecorder) RLockKVStore() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockKVStore", reflect.TypeOf((*MockBackupService)(nil).LockKVStore))
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RLockKVStore", reflect.TypeOf((*MockBackupService)(nil).RLockKVStore))
 }
 
 // UnlockKVStore mocks base method.
-func (m *MockBackupService) UnlockKVStore() {
+func (m *MockBackupService) RUnlockKVStore() {
 	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "UnlockKVStore")
+	m.ctrl.Call(m, "RUnlockKVStore")
 }
 
 // UnlockKVStore indicates an expected call of UnlockKVStore.
 func (mr *MockBackupServiceMockRecorder) UnlockKVStore() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnlockKVStore", reflect.TypeOf((*MockBackupService)(nil).UnlockKVStore))
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RUnlockKVStore", reflect.TypeOf((*MockBackupService)(nil).RUnlockKVStore))
 }
 
 // MockSqlBackupRestoreService is a mock of SqlBackupRestoreService interface.
@@ -127,15 +127,15 @@ func (mr *MockSqlBackupRestoreServiceMockRecorder) BackupSqlStore(ctx, w interfa
 }
 
 // LockSqlStore mocks base method.
-func (m *MockSqlBackupRestoreService) LockSqlStore() {
+func (m *MockSqlBackupRestoreService) RLockSqlStore() {
 	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "LockSqlStore")
+	m.ctrl.Call(m, "RLockSqlStore")
 }
 
 // LockSqlStore indicates an expected call of LockSqlStore.
-func (mr *MockSqlBackupRestoreServiceMockRecorder) LockSqlStore() *gomock.Call {
+func (mr *MockSqlBackupRestoreServiceMockRecorder) RLockSqlStore() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockSqlStore", reflect.TypeOf((*MockSqlBackupRestoreService)(nil).LockSqlStore))
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RLockSqlStore", reflect.TypeOf((*MockSqlBackupRestoreService)(nil).RLockSqlStore))
 }
 
 // RestoreSqlStore mocks base method.
@@ -153,15 +153,15 @@ func (mr *MockSqlBackupRestoreServiceMockRecorder) RestoreSqlStore(ctx, r interf
 }
 
 // UnlockSqlStore mocks base method.
-func (m *MockSqlBackupRestoreService) UnlockSqlStore() {
+func (m *MockSqlBackupRestoreService) RUnlockSqlStore() {
 	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "UnlockSqlStore")
+	m.ctrl.Call(m, "RUnlockSqlStore")
 }
 
 // UnlockSqlStore indicates an expected call of UnlockSqlStore.
-func (mr *MockSqlBackupRestoreServiceMockRecorder) UnlockSqlStore() *gomock.Call {
+func (mr *MockSqlBackupRestoreServiceMockRecorder) RUnlockSqlStore() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnlockSqlStore", reflect.TypeOf((*MockSqlBackupRestoreService)(nil).UnlockSqlStore))
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RUnlockSqlStore", reflect.TypeOf((*MockSqlBackupRestoreService)(nil).RUnlockSqlStore))
 }
 
 // MockBucketManifestWriter is a mock of BucketManifestWriter interface.

mock/kv.go

@@ -28,10 +28,10 @@ func (s *Store) Update(ctx context.Context, fn func(kv.Tx) error) error {
 	return s.UpdateFn(fn)
 }
 
-// Lock and unlock methods are to satisfy the kv.Store interface
+// RLock and RUnlock methods are to satisfy the kv.Store interface
-func (s *Store) Lock() {}
+func (s *Store) RLock() {}
 
-func (s *Store) Unlock() {}
+func (s *Store) RUnlock() {}
 
 func (s *Store) Backup(ctx context.Context, w io.Writer) error {
 	return s.BackupFn(ctx, w)

sqlite/sqlite.go

@@ -28,7 +28,7 @@ const (
 // SqlStore is a wrapper around the db and provides basic functionality for maintaining the db
 // including flushing the data from the db during end-to-end testing.
 type SqlStore struct {
-	Mu   sync.Mutex
+	Mu   sync.RWMutex
 	DB   *sqlx.DB
 	log  *zap.Logger
 	path string
@@ -80,13 +80,13 @@ func (s *SqlStore) Close() error {
 
 // LockSqlStore locks the database using the mutex. This is intended to lock the database for writes.
 // It is the responsibilty of implementing service code to manage locks for write operations.
-func (s *SqlStore) LockSqlStore() {
+func (s *SqlStore) RLockSqlStore() {
-	s.Mu.Lock()
+	s.Mu.RLock()
 }
 
 // UnlockSqlStore unlocks the database.
-func (s *SqlStore) UnlockSqlStore() {
+func (s *SqlStore) RUnlockSqlStore() {
-	s.Mu.Unlock()
+	s.Mu.RUnlock()
 }
 
 // Flush deletes all records for all tables in the database.

storage/engine.go

@@ -81,8 +81,8 @@ type MetaClient interface {
 	RetentionPolicy(database, policy string) (*meta.RetentionPolicyInfo, error)
 	ShardGroupsByTimeRange(database, policy string, min, max time.Time) (a []meta.ShardGroupInfo, err error)
 	UpdateRetentionPolicy(database, name string, rpu *meta.RetentionPolicyUpdate, makeDefault bool) error
-	Lock()
+	RLock()
-	Unlock()
+	RUnlock()
 	Backup(ctx context.Context, w io.Writer) error
 	Restore(ctx context.Context, r io.Reader) error
 	Data() meta.Data
@@ -320,16 +320,16 @@ func (e *Engine) DeleteBucketRangePredicate(ctx context.Context, orgID, bucketID
 	return e.tsdbStore.DeleteSeriesWithPredicate(ctx, bucketID.String(), min, max, pred)
 }
 
-// LockKVStore locks the KV store as well as the engine in preparation for doing a backup.
+// RLockKVStore locks the KV store as well as the engine in preparation for doing a backup.
-func (e *Engine) LockKVStore() {
+func (e *Engine) RLockKVStore() {
-	e.mu.Lock()
+	e.mu.RLock()
-	e.metaClient.Lock()
+	e.metaClient.RLock()
 }
 
-// UnlockKVStore unlocks the KV store & engine, intended to be used after a backup is complete.
+// RUnlockKVStore unlocks the KV store & engine, intended to be used after a backup is complete.
-func (e *Engine) UnlockKVStore() {
+func (e *Engine) RUnlockKVStore() {
-	e.mu.Unlock()
+	e.mu.RUnlock()
-	e.metaClient.Unlock()
+	e.metaClient.RUnlock()
 }
 
 func (e *Engine) BackupKVStore(ctx context.Context, w io.Writer) error {

v1/services/meta/client.go

@@ -1036,12 +1036,12 @@ func (c *Client) Load() error {
 	})
 }
 
-func (c *Client) Lock() {
+func (c *Client) RLock() {
-	c.store.Lock()
+	c.store.RLock()
 }
 
-func (c *Client) Unlock() {
+func (c *Client) RUnlock() {
-	c.store.Unlock()
+	c.store.RUnlock()
 }
 
 func (c *Client) Backup(ctx context.Context, w io.Writer) error {

v1/services/meta/filestore/kv.go

@@ -32,13 +32,12 @@ func (s *KVStore) Update(ctx context.Context, f func(kv.Tx) error) error {
 	return f(&Tx{kv: s, ctx: ctx, writable: true})
 }
 
-// Lock and unlock are only used when doing a backup, so panic if they are called here.
+func (s *KVStore) RLock() {
-func (s *KVStore) Lock() {
+	s.mu.RLock()
-	panic("not implemented")
 }
 
-func (s *KVStore) Unlock() {
+func (s *KVStore) RUnlock() {
-	panic("not implemented")
+	s.mu.RUnlock()
 }
 
 func (s *KVStore) Backup(ctx context.Context, w io.Writer) error {