From ae6655f11e965c983e6b04a5603f4dc2aac9bd87 Mon Sep 17 00:00:00 2001 From: Thomas Gosteli Date: Fri, 1 Nov 2024 12:23:48 +0000 Subject: [PATCH 1/3] fix(defrag): handle no space left error Signed-off-by: Thomas Gosteli --- server/mvcc/backend/backend.go | 12 +++++--- tests/e2e/defrag_no_space_test.go | 49 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 tests/e2e/defrag_no_space_test.go diff --git a/server/mvcc/backend/backend.go b/server/mvcc/backend/backend.go index 7d77da12fd6..077146b82ee 100644 --- a/server/mvcc/backend/backend.go +++ b/server/mvcc/backend/backend.go @@ -477,10 +477,6 @@ func (b *backend) defrag() error { b.readTx.Lock() defer b.readTx.Unlock() - b.batchTx.unsafeCommit(true) - - b.batchTx.tx = nil - // Create a temporary file to ensure we start with a clean slate. // Snapshotter.cleanupSnapdir cleans up any of these that are found during startup. dir := filepath.Dir(b.db.Path()) @@ -488,11 +484,14 @@ func (b *backend) defrag() error { if err != nil { return err } + options := bolt.Options{} if boltOpenOptions != nil { options = *boltOpenOptions } options.OpenFile = func(_ string, _ int, _ os.FileMode) (file *os.File, err error) { + // gofail: var defragNoSpace string + // return nil, fmt.Errorf(defragNoSpace) return temp, nil } // Don't load tmp db into memory regardless of opening options @@ -515,6 +514,11 @@ func (b *backend) defrag() error { zap.String("current-db-size-in-use", humanize.Bytes(uint64(sizeInUse1))), ) } + + // Commit/stop and then reset current transactions (including the readTx) + b.batchTx.unsafeCommit(true) + b.batchTx.tx = nil + // gofail: var defragBeforeCopy struct{} err = defragdb(b.db, tmpdb, defragLimit) if err != nil { diff --git a/tests/e2e/defrag_no_space_test.go b/tests/e2e/defrag_no_space_test.go new file mode 100644 index 00000000000..810136f156e --- /dev/null +++ b/tests/e2e/defrag_no_space_test.go @@ -0,0 +1,49 @@ +// Copyright 2024 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 e2e + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "go.etcd.io/etcd/tests/v3/framework/config" + "go.etcd.io/etcd/tests/v3/framework/e2e" +) + +func TestDefragNoSpace(t *testing.T) { + e2e.BeforeTest(t) + + clus, err := e2e.NewEtcdProcessCluster(context.TODO(), t, + e2e.WithClusterSize(1), + e2e.WithGoFailEnabled(true), + ) + require.NoError(t, err) + t.Cleanup(func() { clus.Stop() }) + + member := clus.Procs[0] + + require.NoError(t, member.Failpoints().SetupHTTP(context.Background(), "defragNoSpace", `return("no space")`)) + require.ErrorContains(t, member.Etcdctl().Defragment(context.Background(), config.DefragOption{Timeout: time.Minute}), "no space") + + // Make sure etcd continues to run even after the failed defrag attempt + require.NoError(t, member.Etcdctl().Put(context.Background(), "foo", "bar", config.PutOptions{})) + value, err := member.Etcdctl().Get(context.Background(), "foo", config.GetOptions{}) + require.NoError(t, err) + require.Len(t, value.Kvs, 1) + require.Equal(t, "bar", string(value.Kvs[0].Value)) +} From 1d175079a8d094eaee2e1e09e181ce69d82def7b Mon Sep 17 00:00:00 2001 From: Thomas Gosteli Date: Wed, 6 Nov 2024 11:47:18 +0100 Subject: [PATCH 2/3] fix(defrag): handle defragdb failure Signed-off-by: Thomas Gosteli --- server/mvcc/backend/backend.go | 12 +++++- tests/e2e/defrag_no_space_test.go | 62 +++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/server/mvcc/backend/backend.go b/server/mvcc/backend/backend.go index 077146b82ee..d04126d94ae 100644 --- a/server/mvcc/backend/backend.go +++ b/server/mvcc/backend/backend.go @@ -490,8 +490,8 @@ func (b *backend) defrag() error { options = *boltOpenOptions } options.OpenFile = func(_ string, _ int, _ os.FileMode) (file *os.File, err error) { - // gofail: var defragNoSpace string - // return nil, fmt.Errorf(defragNoSpace) + // gofail: var defragOpenFileError string + // return nil, fmt.Errorf(defragOpenFileError) return temp, nil } // Don't load tmp db into memory regardless of opening options @@ -526,6 +526,11 @@ func (b *backend) defrag() error { if rmErr := os.RemoveAll(tmpdb.Path()); rmErr != nil { b.lg.Error("failed to remove db.tmp after defragmentation completed", zap.Error(rmErr)) } + + // restore the bbolt transactions if defragmentation fails + b.batchTx.tx = b.unsafeBegin(true) + b.readTx.tx = b.unsafeBegin(false) + return err } @@ -578,6 +583,9 @@ func (b *backend) defrag() error { } func defragdb(odb, tmpdb *bolt.DB, limit int) error { + // gofail: var defragdbFail string + // return fmt.Errorf(defragdbFail) + // open a tx on tmpdb for writes tmptx, err := tmpdb.Begin(true) if err != nil { diff --git a/tests/e2e/defrag_no_space_test.go b/tests/e2e/defrag_no_space_test.go index 810136f156e..f6ceabe667b 100644 --- a/tests/e2e/defrag_no_space_test.go +++ b/tests/e2e/defrag_no_space_test.go @@ -16,6 +16,7 @@ package e2e import ( "context" + "fmt" "testing" "time" @@ -26,24 +27,45 @@ import ( ) func TestDefragNoSpace(t *testing.T) { - e2e.BeforeTest(t) - - clus, err := e2e.NewEtcdProcessCluster(context.TODO(), t, - e2e.WithClusterSize(1), - e2e.WithGoFailEnabled(true), - ) - require.NoError(t, err) - t.Cleanup(func() { clus.Stop() }) - - member := clus.Procs[0] - - require.NoError(t, member.Failpoints().SetupHTTP(context.Background(), "defragNoSpace", `return("no space")`)) - require.ErrorContains(t, member.Etcdctl().Defragment(context.Background(), config.DefragOption{Timeout: time.Minute}), "no space") - - // Make sure etcd continues to run even after the failed defrag attempt - require.NoError(t, member.Etcdctl().Put(context.Background(), "foo", "bar", config.PutOptions{})) - value, err := member.Etcdctl().Get(context.Background(), "foo", config.GetOptions{}) - require.NoError(t, err) - require.Len(t, value.Kvs, 1) - require.Equal(t, "bar", string(value.Kvs[0].Value)) + tests := []struct { + name string + failpoint string + err string + }{ + { + name: "no space (#18810) - can't open/create new bbolt db", + failpoint: "defragOpenFileError", + err: "no space", + }, + { + name: "defragdb failure", + failpoint: "defragdbFail", + err: "some random error", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + e2e.BeforeTest(t) + + clus, err := e2e.NewEtcdProcessCluster(context.TODO(), t, + e2e.WithClusterSize(1), + e2e.WithGoFailEnabled(true), + ) + require.NoError(t, err) + t.Cleanup(func() { clus.Stop() }) + + member := clus.Procs[0] + + require.NoError(t, member.Failpoints().SetupHTTP(context.Background(), tc.failpoint, fmt.Sprintf(`return("%s")`, tc.err))) + require.ErrorContains(t, member.Etcdctl().Defragment(context.Background(), config.DefragOption{Timeout: time.Minute}), tc.err) + + // Make sure etcd continues to run even after the failed defrag attempt + require.NoError(t, member.Etcdctl().Put(context.Background(), "foo", "bar", config.PutOptions{})) + value, err := member.Etcdctl().Get(context.Background(), "foo", config.GetOptions{}) + require.NoError(t, err) + require.Len(t, value.Kvs, 1) + require.Equal(t, "bar", string(value.Kvs[0].Value)) + }) + } } From f26ff912a98d08d8c9ac94563a99051c75c4a493 Mon Sep 17 00:00:00 2001 From: Thomas Gosteli Date: Wed, 6 Nov 2024 14:15:07 +0100 Subject: [PATCH 3/3] chore(e2e): adapt defrag tests for 3.5 Signed-off-by: Thomas Gosteli --- tests/e2e/defrag_no_space_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/e2e/defrag_no_space_test.go b/tests/e2e/defrag_no_space_test.go index f6ceabe667b..1f5c2b50894 100644 --- a/tests/e2e/defrag_no_space_test.go +++ b/tests/e2e/defrag_no_space_test.go @@ -22,7 +22,6 @@ import ( "github.com/stretchr/testify/require" - "go.etcd.io/etcd/tests/v3/framework/config" "go.etcd.io/etcd/tests/v3/framework/e2e" ) @@ -48,21 +47,25 @@ func TestDefragNoSpace(t *testing.T) { t.Run(tc.name, func(t *testing.T) { e2e.BeforeTest(t) - clus, err := e2e.NewEtcdProcessCluster(context.TODO(), t, - e2e.WithClusterSize(1), - e2e.WithGoFailEnabled(true), + clus, err := e2e.NewEtcdProcessCluster(t, + &e2e.EtcdProcessClusterConfig{ + ClusterSize: 1, + LogLevel: "debug", + GoFailEnabled: true, + }, ) require.NoError(t, err) t.Cleanup(func() { clus.Stop() }) member := clus.Procs[0] + etcdctl := member.Etcdctl(e2e.ClientNonTLS, false, false) require.NoError(t, member.Failpoints().SetupHTTP(context.Background(), tc.failpoint, fmt.Sprintf(`return("%s")`, tc.err))) - require.ErrorContains(t, member.Etcdctl().Defragment(context.Background(), config.DefragOption{Timeout: time.Minute}), tc.err) + require.ErrorContains(t, etcdctl.Defragment(time.Minute), tc.err) // Make sure etcd continues to run even after the failed defrag attempt - require.NoError(t, member.Etcdctl().Put(context.Background(), "foo", "bar", config.PutOptions{})) - value, err := member.Etcdctl().Get(context.Background(), "foo", config.GetOptions{}) + require.NoError(t, etcdctl.Put("foo", "bar")) + value, err := etcdctl.Get("foo") require.NoError(t, err) require.Len(t, value.Kvs, 1) require.Equal(t, "bar", string(value.Kvs[0].Value))