diff --git a/server/mvcc/backend/batch_tx.go b/server/mvcc/backend/batch_tx.go index b2b0ad7cbf0..a8e64919916 100644 --- a/server/mvcc/backend/batch_tx.go +++ b/server/mvcc/backend/batch_tx.go @@ -53,6 +53,8 @@ type BatchTx interface { Commit() // CommitAndStop commits the previous tx and does not create a new one. CommitAndStop() + LockInsideApply() + LockOutsideApply() } type batchTx struct { @@ -67,6 +69,16 @@ func (t *batchTx) Lock() { t.Mutex.Lock() } +func (t *batchTx) LockInsideApply() { + ValidateCalledInsideApply(t.backend.lg) + t.Lock() +} + +func (t *batchTx) LockOutsideApply() { + ValidateCalledOutSideApply(t.backend.lg) + t.Lock() +} + func (t *batchTx) Unlock() { if t.pending >= t.backend.batchLimit { t.commit(false) diff --git a/server/mvcc/backend/verify.go b/server/mvcc/backend/verify.go new file mode 100644 index 00000000000..2f3dc0221c6 --- /dev/null +++ b/server/mvcc/backend/verify.go @@ -0,0 +1,56 @@ +// Copyright 2022 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package backend + +import ( + "os" + "runtime/debug" + "strings" + + "go.uber.org/zap" +) + +const ( + ENV_VERIFY = "ETCD_VERIFY" + ENV_VERIFY_ALL_VALUE = "all" + ENV_VERIFY_LOCK = "lock" +) + +func ValidateCalledInsideApply(lg *zap.Logger) { + if !verifyLockEnabled() { + return + } + if !insideApply() { + lg.Panic("Called outside of APPLY!", zap.Stack("stacktrace")) + } +} + +func ValidateCalledOutSideApply(lg *zap.Logger) { + if !verifyLockEnabled() { + return + } + if insideApply() { + lg.Panic("Called inside of APPLY!", zap.Stack("stacktrace")) + } +} + +func verifyLockEnabled() bool { + return os.Getenv(ENV_VERIFY) == ENV_VERIFY_ALL_VALUE || os.Getenv(ENV_VERIFY) == ENV_VERIFY_LOCK +} + +func insideApply() bool { + stackTraceStr := string(debug.Stack()) + return strings.Contains(stackTraceStr, ".applyEntries") +} diff --git a/server/mvcc/backend/verify_test.go b/server/mvcc/backend/verify_test.go new file mode 100644 index 00000000000..f4f4db47527 --- /dev/null +++ b/server/mvcc/backend/verify_test.go @@ -0,0 +1,91 @@ +// Copyright 2022 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package backend_test + +import ( + "fmt" + "os" + "testing" + "time" + + "go.etcd.io/etcd/server/v3/mvcc/backend" + betesting "go.etcd.io/etcd/server/v3/mvcc/backend/testing" +) + +func TestLockVerify(t *testing.T) { + tcs := []struct { + insideApply bool + lock func(tx backend.BatchTx) + expectPanic bool + }{ + { + insideApply: true, + lock: lockInsideApply, + expectPanic: false, + }, + { + insideApply: false, + lock: lockInsideApply, + expectPanic: true, + }, + { + insideApply: false, + lock: lockOutsideApply, + expectPanic: false, + }, + { + insideApply: true, + lock: lockOutsideApply, + expectPanic: true, + }, + } + env := os.Getenv("ETCD_VERIFY") + os.Setenv("ETCD_VERIFY", "lock") + defer func() { + os.Setenv("ETCD_VERIFY", env) + }() + for i, tc := range tcs { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + + be, _ := betesting.NewTmpBackend(t, time.Hour, 10000) + + hasPaniced := handlePanic(func() { + if tc.insideApply { + applyEntries(be, tc.lock) + } else { + tc.lock(be.BatchTx()) + } + }) != nil + if hasPaniced != tc.expectPanic { + t.Errorf("%v != %v", hasPaniced, tc.expectPanic) + } + }) + } +} + +func handlePanic(f func()) (result interface{}) { + defer func() { + result = recover() + }() + f() + return result +} + +func applyEntries(be backend.Backend, f func(tx backend.BatchTx)) { + f(be.BatchTx()) +} + +func lockInsideApply(tx backend.BatchTx) { tx.LockInsideApply() } +func lockOutsideApply(tx backend.BatchTx) { tx.LockOutsideApply() } diff --git a/server/mvcc/kvstore_test.go b/server/mvcc/kvstore_test.go index 1e56674d08b..d2a9e55e89f 100644 --- a/server/mvcc/kvstore_test.go +++ b/server/mvcc/kvstore_test.go @@ -894,8 +894,10 @@ func (b *fakeBatchTx) UnsafeDelete(bucket backend.Bucket, key []byte) { func (b *fakeBatchTx) UnsafeForEach(bucket backend.Bucket, visitor func(k, v []byte) error) error { return nil } -func (b *fakeBatchTx) Commit() {} -func (b *fakeBatchTx) CommitAndStop() {} +func (b *fakeBatchTx) Commit() {} +func (b *fakeBatchTx) CommitAndStop() {} +func (b *fakeBatchTx) LockInsideApply() {} +func (b *fakeBatchTx) LockOutsideApply() {} type fakeBackend struct { tx *fakeBatchTx diff --git a/test.sh b/test.sh index 308cde40215..4e69bd65671 100755 --- a/test.sh +++ b/test.sh @@ -43,6 +43,7 @@ set -o pipefail # e.g. add/update missing dependencies. Such divergences should be # detected and trigger a failure that needs explicit developer's action. export GOFLAGS=-mod=readonly +export ETCD_VERIFY=all source ./scripts/test_lib.sh source ./build.sh