Skip to content

Commit

Permalink
server: Remove lock from adapter to avoid deadlock
Browse files Browse the repository at this point in the history
  • Loading branch information
serathius committed Oct 21, 2021
1 parent 5d5f681 commit 7a5e622
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 45 deletions.
31 changes: 8 additions & 23 deletions server/etcdserver/adapters.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,17 @@ import (
"go.etcd.io/etcd/api/v3/membershippb"
"go.etcd.io/etcd/api/v3/version"
serverversion "go.etcd.io/etcd/server/v3/etcdserver/version"
"go.etcd.io/etcd/server/v3/storage/backend"
"go.etcd.io/etcd/server/v3/storage/schema"
)

// serverVersionAdapter implements Server interface needed by serverversion.Monitor
type serverVersionAdapter struct {
*EtcdServer
tx backend.BatchTx
}

func newServerVersionAdapter(s *EtcdServer) *serverVersionAdapter {
return &serverVersionAdapter{
EtcdServer: s,
tx: nil,
}
}

Expand Down Expand Up @@ -75,31 +72,19 @@ func (s *serverVersionAdapter) GetMembersVersions() map[string]*version.Versions
}

func (s *serverVersionAdapter) GetStorageVersion() *semver.Version {
if s.tx == nil {
s.Lock()
defer s.Unlock()
}
v, err := schema.UnsafeDetectSchemaVersion(s.lg, s.tx)
tx := s.be.BatchTx()
tx.Lock()
defer tx.Unlock()
v, err := schema.UnsafeDetectSchemaVersion(s.lg, tx)
if err != nil {
return nil
}
return &v
}

func (s *serverVersionAdapter) UpdateStorageVersion(target semver.Version) error {
if s.tx == nil {
s.Lock()
defer s.Unlock()
}
return schema.UnsafeMigrate(s.lg, s.tx, s.r.storage, target)
}

func (s *serverVersionAdapter) Lock() {
s.tx = s.be.BatchTx()
s.tx.Lock()
}

func (s *serverVersionAdapter) Unlock() {
s.tx.Unlock()
s.tx = nil
tx := s.be.BatchTx()
tx.Lock()
defer tx.Unlock()
return schema.UnsafeMigrate(s.lg, tx, s.r.storage, target)
}
5 changes: 0 additions & 5 deletions server/etcdserver/version/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ type Server interface {

GetStorageVersion() *semver.Version
UpdateStorageVersion(semver.Version) error

Lock()
Unlock()
}

func NewMonitor(lg *zap.Logger, storage Server) *Monitor {
Expand Down Expand Up @@ -95,8 +92,6 @@ func (m *Monitor) UpdateStorageVersionIfNeeded() {
if cv == nil {
return
}
m.s.Lock()
defer m.s.Unlock()
sv := m.s.GetStorageVersion()

if sv == nil || sv.Major != cv.Major || sv.Minor != cv.Minor {
Expand Down
11 changes: 0 additions & 11 deletions server/etcdserver/version/monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,3 @@ func (s *storageMock) UpdateStorageVersion(v semver.Version) error {
s.storageVersion = &v
return nil
}

func (s *storageMock) Lock() {
if s.locked {
panic("Deadlock")
}
s.locked = true
}

func (s *storageMock) Unlock() {
s.locked = false
}
6 changes: 0 additions & 6 deletions server/etcdserver/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,3 @@ func (m *memberMock) UpdateStorageVersion(v semver.Version) error {

func (m *memberMock) TriggerSnapshot() {
}

func (m *memberMock) Lock() {
}

func (m *memberMock) Unlock() {
}

0 comments on commit 7a5e622

Please sign in to comment.