From bc97a94564fe279d397a4dff3d884b3f68dbbd40 Mon Sep 17 00:00:00 2001 From: caojiamingalan Date: Fri, 14 Jul 2023 20:08:25 -0500 Subject: [PATCH] Follow up https://github.com/etcd-io/etcd/pull/16068#discussion_r1263664700 Replace unnecessary Lock()/Unlock()s with RLock()/RUnlock()s Signed-off-by: caojiamingalan --- server/storage/backend/read_tx.go | 5 +++++ server/storage/mvcc/kvstore.go | 10 +++++----- server/storage/schema/alarm.go | 4 ++-- server/storage/schema/membership.go | 4 ++-- server/storage/schema/schema.go | 4 ++-- server/storage/schema/version.go | 4 ++-- 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/server/storage/backend/read_tx.go b/server/storage/backend/read_tx.go index 56327d52ae6..592e301a0f6 100644 --- a/server/storage/backend/read_tx.go +++ b/server/storage/backend/read_tx.go @@ -26,8 +26,13 @@ import ( // is known to never overwrite any key so range is safe. type ReadTx interface { + // Lock and Unlock should only be used in the following three cases: + // 1. When committing a transaction. Because it will reset the read tx, including the buffer; + // 2. When writing the pending data back to read buf, because obviously no reading is allowed when writing the read buffer; + // 3. When performing defragmentation, because again it will reset the read tx, including the read buffer. Lock() Unlock() + // RLock and RUnlock should be used for all other cases. RLock() RUnlock() diff --git a/server/storage/mvcc/kvstore.go b/server/storage/mvcc/kvstore.go index eeb82c68bc9..d4c44e42182 100644 --- a/server/storage/mvcc/kvstore.go +++ b/server/storage/mvcc/kvstore.go @@ -228,8 +228,8 @@ func (s *store) updateCompactRev(rev int64) (<-chan struct{}, int64, error) { // checkPrevCompactionCompleted checks whether the previous scheduled compaction is completed. func (s *store) checkPrevCompactionCompleted() bool { tx := s.b.ReadTx() - tx.Lock() - defer tx.Unlock() + tx.RLock() + defer tx.RUnlock() scheduledCompact, scheduledCompactFound := UnsafeReadScheduledCompact(tx) finishedCompact, finishedCompactFound := UnsafeReadFinishedCompact(tx) return scheduledCompact == finishedCompact && scheduledCompactFound == finishedCompactFound @@ -328,7 +328,7 @@ func (s *store) restore() error { // restore index tx := s.b.ReadTx() - tx.Lock() + tx.RLock() finishedCompact, found := UnsafeReadFinishedCompact(tx) if found { @@ -384,7 +384,7 @@ func (s *store) restore() error { for key, lid := range keyToLease { if s.le == nil { - tx.Unlock() + tx.RUnlock() panic("no lessor to attach lease") } err := s.le.Attach(lid, []lease.LeaseItem{{Key: key}}) @@ -397,7 +397,7 @@ func (s *store) restore() error { } } - tx.Unlock() + tx.RUnlock() s.lg.Info("kvstore restored", zap.Int64("current-rev", s.currentRev)) diff --git a/server/storage/schema/alarm.go b/server/storage/schema/alarm.go index f1d80b27a6b..66468a57e30 100644 --- a/server/storage/schema/alarm.go +++ b/server/storage/schema/alarm.go @@ -74,8 +74,8 @@ func (s *alarmBackend) mustUnsafeDeleteAlarm(tx backend.BatchTx, alarm *etcdserv func (s *alarmBackend) GetAllAlarms() ([]*etcdserverpb.AlarmMember, error) { tx := s.be.ReadTx() - tx.Lock() - defer tx.Unlock() + tx.RLock() + defer tx.RUnlock() return s.unsafeGetAllAlarms(tx) } diff --git a/server/storage/schema/membership.go b/server/storage/schema/membership.go index a42353c4168..137d2785071 100644 --- a/server/storage/schema/membership.go +++ b/server/storage/schema/membership.go @@ -207,8 +207,8 @@ func (s *membershipBackend) ClusterVersionFromBackend() *semver.Version { func (s *membershipBackend) DowngradeInfoFromBackend() *version.DowngradeInfo { dkey := ClusterDowngradeKeyName tx := s.be.ReadTx() - tx.Lock() - defer tx.Unlock() + tx.RLock() + defer tx.RUnlock() keys, vals := tx.UnsafeRange(Cluster, dkey, nil, 0) if len(keys) == 0 { return nil diff --git a/server/storage/schema/schema.go b/server/storage/schema/schema.go index e2e7fe7e566..a10f974a045 100644 --- a/server/storage/schema/schema.go +++ b/server/storage/schema/schema.go @@ -27,8 +27,8 @@ import ( // Validate checks provided backend to confirm that schema used is supported. func Validate(lg *zap.Logger, tx backend.ReadTx) error { - tx.Lock() - defer tx.Unlock() + tx.RLock() + defer tx.RUnlock() return unsafeValidate(lg, tx) } diff --git a/server/storage/schema/version.go b/server/storage/schema/version.go index 18f566a993e..d8c7a222ec8 100644 --- a/server/storage/schema/version.go +++ b/server/storage/schema/version.go @@ -24,8 +24,8 @@ import ( // ReadStorageVersion loads storage version from given backend transaction. // Populated since v3.6 func ReadStorageVersion(tx backend.ReadTx) *semver.Version { - tx.Lock() - defer tx.Unlock() + tx.RLock() + defer tx.RUnlock() return UnsafeReadStorageVersion(tx) }