From dc3b501298fe133d03342b5f606e6645a87e0697 Mon Sep 17 00:00:00 2001
From: Daniel Moran <danxmoran@gmail.com>
Date: Tue, 31 Aug 2021 17:03:54 -0400
Subject: [PATCH] refactor: rename Lock/Unlock on KV stores to RLock/RUnlock
 (#22357)

---
 authorizer/backup.go             |  8 ++++----
 authorizer/sql_backup_restore.go |  8 ++++----
 backup.go                        | 16 ++++++++--------
 bolt/kv.go                       |  6 ++----
 cmd/influxd/launcher/engine.go   |  8 ++++----
 http/backup_service.go           |  8 ++++----
 http/backup_service_test.go      | 14 ++++----------
 inmem/kv.go                      |  9 ++++-----
 kv/store.go                      |  8 ++++----
 mock/backup_service.go           | 30 +++++++++++++++---------------
 mock/kv.go                       |  6 +++---
 sqlite/sqlite.go                 | 10 +++++-----
 storage/engine.go                | 20 ++++++++++----------
 v1/services/meta/client.go       |  8 ++++----
 v1/services/meta/filestore/kv.go |  9 ++++-----
 15 files changed, 79 insertions(+), 89 deletions(-)

diff --git a/authorizer/backup.go b/authorizer/backup.go
index d1fb881948..dbbe94c91d 100644
--- a/authorizer/backup.go
+++ b/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() {
-	b.s.LockKVStore()
+func (b BackupService) RLockKVStore() {
+	b.s.RLockKVStore()
 }
 
-func (b BackupService) UnlockKVStore() {
-	b.s.UnlockKVStore()
+func (b BackupService) RUnlockKVStore() {
+	b.s.RUnlockKVStore()
 }
diff --git a/authorizer/sql_backup_restore.go b/authorizer/sql_backup_restore.go
index 6d0ab869b9..906b0b5740 100644
--- a/authorizer/sql_backup_restore.go
+++ b/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() {
-	s.s.LockSqlStore()
+func (s SqlBackupRestoreService) RLockSqlStore() {
+	s.s.RLockSqlStore()
 }
 
-func (s SqlBackupRestoreService) UnlockSqlStore() {
-	s.s.UnlockSqlStore()
+func (s SqlBackupRestoreService) RUnlockSqlStore() {
+	s.s.RUnlockSqlStore()
 }
diff --git a/backup.go b/backup.go
index e187a3325b..c5f4af6b70 100644
--- a/backup.go
+++ b/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.
-	LockKVStore()
+	// RLockKVStore locks the database.
+	RLockKVStore()
 
-	// UnlockKVStore unlocks the database.
-	UnlockKVStore()
+	// RUnlockKVStore unlocks the database.
+	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.
-	LockSqlStore()
+	// RLockSqlStore takes a read lock on the database
+	RLockSqlStore()
 
-	// UnlockSqlStore unlocks the database.
-	UnlockSqlStore()
+	// RUnlockSqlStore releases a previously-taken read lock on the database.
+	RUnlockSqlStore()
 }
 
 type BucketManifestWriter interface {
diff --git a/bolt/kv.go b/bolt/kv.go
index 355c260faf..85f99f0828 100644
--- a/bolt/kv.go
+++ b/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) Lock() {
+func (s *KVStore) RLock() {
 	s.mu.RLock()
 }
 
-// UnlockKVStore removes the read lock used during a backup.
-func (s *KVStore) Unlock() {
+func (s *KVStore) RUnlock() {
 	s.mu.RUnlock()
 }
 
diff --git a/cmd/influxd/launcher/engine.go b/cmd/influxd/launcher/engine.go
index 14e8dfe1cb..1ee720547d 100644
--- a/cmd/influxd/launcher/engine.go
+++ b/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() {
-	t.engine.LockKVStore()
+func (t *TemporaryEngine) RLockKVStore() {
+	t.engine.RLockKVStore()
 }
 
-func (t *TemporaryEngine) UnlockKVStore() {
-	t.engine.UnlockKVStore()
+func (t *TemporaryEngine) RUnlockKVStore() {
+	t.engine.RUnlockKVStore()
 }
 
 func (t *TemporaryEngine) RestoreKVStore(ctx context.Context, r io.Reader) error {
diff --git a/http/backup_service.go b/http/backup_service.go
index b7802ecb3e..dadbf2d902 100644
--- a/http/backup_service.go
+++ b/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()
-	defer h.BackupService.UnlockKVStore()
+	h.BackupService.RLockKVStore()
+	defer h.BackupService.RUnlockKVStore()
 
-	h.SqlBackupRestoreService.LockSqlStore()
-	defer h.SqlBackupRestoreService.UnlockSqlStore()
+	h.SqlBackupRestoreService.RLockSqlStore()
+	defer h.SqlBackupRestoreService.RUnlockSqlStore()
 
 	dataWriter := multipart.NewWriter(w)
 	w.Header().Set("Content-Type", "multipart/mixed; boundary="+dataWriter.Boundary())
diff --git a/http/backup_service_test.go b/http/backup_service_test.go
index 07fceac127..b292271740 100644
--- a/http/backup_service_test.go
+++ b/http/backup_service_test.go
@@ -38,21 +38,15 @@ func TestBackupMetaService(t *testing.T) {
 		BackupKVStore(gomock.Any(), gomock.Any()).
 		Return(nil)
 
-	backupSvc.EXPECT().
-		LockKVStore()
-
-	backupSvc.EXPECT().
-		UnlockKVStore()
+	backupSvc.EXPECT().RLockKVStore()
+	backupSvc.EXPECT().UnlockKVStore()
 
 	sqlBackupSvc.EXPECT().
 		BackupSqlStore(gomock.Any(), gomock.Any()).
 		Return(nil)
 
-	sqlBackupSvc.EXPECT().
-		LockSqlStore()
-
-	sqlBackupSvc.EXPECT().
-		UnlockSqlStore()
+	sqlBackupSvc.EXPECT().RLockSqlStore()
+	sqlBackupSvc.EXPECT().RUnlockSqlStore()
 
 	bucketManifestWriter.EXPECT().
 		WriteManifest(gomock.Any(), gomock.Any()).
diff --git a/inmem/kv.go b/inmem/kv.go
index ade12462cd..38800cc5f8 100644
--- a/inmem/kv.go
+++ b/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) Lock() {
-	panic("not implemented")
+func (s *KVStore) RLock() {
+	s.mu.RLock()
 }
 
-func (s *KVStore) Unlock() {
-	panic("not implemented")
+func (s *KVStore) RUnlock() {
+	s.mu.RUnlock()
 }
 
 func (s *KVStore) Backup(ctx context.Context, w io.Writer) error {
diff --git a/kv/store.go b/kv/store.go
index 3d2e06a76f..25cc619d94 100644
--- a/kv/store.go
+++ b/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
-	Lock()
-	// Unlock unlocks the underlying KV store for writes
-	Unlock()
+	// RLock takes a read lock on the underlying KV store.
+	RLock()
+	// RUnlock releases a previously-taken read lock
+	RUnlock()
 }
 
 // Tx is a transaction in the store.
diff --git a/mock/backup_service.go b/mock/backup_service.go
index d36cb95246..0fe178647d 100644
--- a/mock/backup_service.go
+++ b/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.
diff --git a/mock/kv.go b/mock/kv.go
index d21e7faf67..44fa84f6a9 100644
--- a/mock/kv.go
+++ b/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
-func (s *Store) Lock() {}
+// RLock and RUnlock methods are to satisfy the kv.Store interface
+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)
diff --git a/sqlite/sqlite.go b/sqlite/sqlite.go
index 8f6ee0aba4..19b16793d1 100644
--- a/sqlite/sqlite.go
+++ b/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() {
-	s.Mu.Lock()
+func (s *SqlStore) RLockSqlStore() {
+	s.Mu.RLock()
 }
 
 // UnlockSqlStore unlocks the database.
-func (s *SqlStore) UnlockSqlStore() {
-	s.Mu.Unlock()
+func (s *SqlStore) RUnlockSqlStore() {
+	s.Mu.RUnlock()
 }
 
 // Flush deletes all records for all tables in the database.
diff --git a/storage/engine.go b/storage/engine.go
index 0cc5d73c23..214dff5bca 100644
--- a/storage/engine.go
+++ b/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()
-	Unlock()
+	RLock()
+	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.
-func (e *Engine) LockKVStore() {
-	e.mu.Lock()
-	e.metaClient.Lock()
+// RLockKVStore locks the KV store as well as the engine in preparation for doing a backup.
+func (e *Engine) RLockKVStore() {
+	e.mu.RLock()
+	e.metaClient.RLock()
 }
 
-// UnlockKVStore unlocks the KV store & engine, intended to be used after a backup is complete.
-func (e *Engine) UnlockKVStore() {
-	e.mu.Unlock()
-	e.metaClient.Unlock()
+// RUnlockKVStore unlocks the KV store & engine, intended to be used after a backup is complete.
+func (e *Engine) RUnlockKVStore() {
+	e.mu.RUnlock()
+	e.metaClient.RUnlock()
 }
 
 func (e *Engine) BackupKVStore(ctx context.Context, w io.Writer) error {
diff --git a/v1/services/meta/client.go b/v1/services/meta/client.go
index c6ff180661..6ad38610f7 100644
--- a/v1/services/meta/client.go
+++ b/v1/services/meta/client.go
@@ -1036,12 +1036,12 @@ func (c *Client) Load() error {
 	})
 }
 
-func (c *Client) Lock() {
-	c.store.Lock()
+func (c *Client) RLock() {
+	c.store.RLock()
 }
 
-func (c *Client) Unlock() {
-	c.store.Unlock()
+func (c *Client) RUnlock() {
+	c.store.RUnlock()
 }
 
 func (c *Client) Backup(ctx context.Context, w io.Writer) error {
diff --git a/v1/services/meta/filestore/kv.go b/v1/services/meta/filestore/kv.go
index 990252ddcc..1f113f4d87 100644
--- a/v1/services/meta/filestore/kv.go
+++ b/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) Lock() {
-	panic("not implemented")
+func (s *KVStore) RLock() {
+	s.mu.RLock()
 }
 
-func (s *KVStore) Unlock() {
-	panic("not implemented")
+func (s *KVStore) RUnlock() {
+	s.mu.RUnlock()
 }
 
 func (s *KVStore) Backup(ctx context.Context, w io.Writer) error {