Skip to content

Commit 66c9cab

Browse files
jcechacejcechace
authored
Merge pull request #1231 from jcechace/PBM-1645-config-resync-triggers
PPBM-1645 Trigger storage resync only when storage identity changes
2 parents 042e6f3 + 785d4c3 commit 66c9cab

File tree

5 files changed

+124
-35
lines changed

5 files changed

+124
-35
lines changed

‎cmd/pbm/config.go‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"fmt"
66
"log"
77
"os"
8-
"reflect"
9-
"strings"
108
"time"
119

1210
"go.mongodb.org/mongo-driver/mongo"
@@ -62,31 +60,30 @@ func runConfig(
6260

6361
switch {
6462
case len(c.set) > 0:
63+
oldCfg, err := pbm.GetConfig(ctx)
64+
if err != nil {
65+
if !errors.Is(err, mongo.ErrNoDocuments) {
66+
return nil, errors.Wrap(err, "unable to get current config")
67+
}
68+
oldCfg = &config.Config{}
69+
}
70+
6571
var o confVals
66-
rsnc := false
6772
for k, v := range c.set {
6873
err := config.SetConfigVar(ctx, conn, k, v)
6974
if err != nil {
7075
return nil, errors.Wrapf(err, "set %s", k)
7176
}
7277
o = append(o, confKV{k, v})
78+
}
7379

74-
path := strings.Split(k, ".")
75-
if !rsnc && len(path) > 0 && path[0] == "storage" {
76-
rsnc = true
77-
}
80+
newCfg, err := pbm.GetConfig(ctx)
81+
if err != nil {
82+
return nil, errors.Wrap(err, "unable to get updated config")
7883
}
79-
if rsnc {
80-
cid, err := pbm.SyncFromStorage(ctx, false)
81-
if err != nil {
82-
return nil, errors.Wrap(err, "resync")
83-
}
8484

85-
if c.wait {
86-
if err := waitForResyncWithTimeout(ctx, pbm, cid, c.waitTime); err != nil {
87-
return nil, err
88-
}
89-
}
85+
if err := resyncIfNeeded(ctx, pbm, oldCfg, newCfg, c); err != nil {
86+
return nil, err
9087
}
9188
return o, nil
9289
case len(c.key) > 0:
@@ -138,18 +135,8 @@ func runConfig(
138135
return nil, errors.Wrap(err, "unable to set config: write to db")
139136
}
140137

141-
// resync storage only if Storage options have changed
142-
if !reflect.DeepEqual(newCfg.Storage, oldCfg.Storage) {
143-
cid, err := pbm.SyncFromStorage(ctx, false)
144-
if err != nil {
145-
return nil, errors.Wrap(err, "resync")
146-
}
147-
148-
if c.wait {
149-
if err := waitForResyncWithTimeout(ctx, pbm, cid, c.waitTime); err != nil {
150-
return nil, err
151-
}
152-
}
138+
if err := resyncIfNeeded(ctx, pbm, oldCfg, newCfg, c); err != nil {
139+
return nil, err
153140
}
154141

155142
return newCfg, nil
@@ -171,3 +158,20 @@ func readConfigFromFile(filename string) (*config.Config, error) {
171158

172159
return config.Parse(file)
173160
}
161+
162+
func resyncIfNeeded(ctx context.Context, pbm *sdk.Client, oldCfg, newCfg *config.Config, c *configOpts) error {
163+
if newCfg.Storage.IsSameStorage(&oldCfg.Storage) {
164+
return nil
165+
}
166+
167+
cid, err := pbm.SyncFromStorage(ctx, false)
168+
if err != nil {
169+
return errors.Wrap(err, "resync")
170+
}
171+
172+
if !c.wait {
173+
return nil
174+
}
175+
176+
return waitForResyncWithTimeout(ctx, pbm, cid, c.waitTime)
177+
}

‎pbm/config/config_test.go‎

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"github.com/google/go-cmp/cmp"
12+
"github.com/percona/percona-backup-mongodb/pbm/storage/oss"
1213
"github.com/testcontainers/testcontainers-go"
1314
"github.com/testcontainers/testcontainers-go/modules/mongodb"
1415
"go.mongodb.org/mongo-driver/mongo"
@@ -43,9 +44,10 @@ func TestIsSameStorage(t *testing.T) {
4344
InsecureSkipTLSVerify: false,
4445
}
4546
eq := &s3.Config{
46-
Region: "eu",
47-
Bucket: "b1",
48-
Prefix: "p1",
47+
Region: "eu",
48+
Bucket: "b1",
49+
Prefix: "p1",
50+
EndpointURL: "ep.com",
4951
}
5052
if !cfg.IsSameStorage(eq) {
5153
t.Errorf("config storage should identify the same instance: cfg=%+v, eq=%+v", cfg, eq)
@@ -68,6 +70,12 @@ func TestIsSameStorage(t *testing.T) {
6870
if cfg.IsSameStorage(neq) {
6971
t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
7072
}
73+
74+
neq = cfg.Clone()
75+
neq.EndpointURL = "ep2.com"
76+
if cfg.IsSameStorage(neq) {
77+
t.Errorf("storage instances has different EndpointURL: cfg=%+v, eq=%+v", cfg, neq)
78+
}
7179
})
7280

7381
t.Run("Azure", func(t *testing.T) {
@@ -82,9 +90,10 @@ func TestIsSameStorage(t *testing.T) {
8290
}
8391

8492
eq := &azure.Config{
85-
Account: "a1",
86-
Container: "c1",
87-
Prefix: "p1",
93+
Account: "a1",
94+
Container: "c1",
95+
Prefix: "p1",
96+
EndpointURL: "az.com",
8897
}
8998
if !cfg.IsSameStorage(eq) {
9099
t.Errorf("config storage should identify the same instance: cfg=%+v, eq=%+v", cfg, eq)
@@ -107,6 +116,12 @@ func TestIsSameStorage(t *testing.T) {
107116
if cfg.IsSameStorage(neq) {
108117
t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
109118
}
119+
120+
neq = cfg.Clone()
121+
neq.EndpointURL = "az2.com"
122+
if cfg.IsSameStorage(neq) {
123+
t.Errorf("storage instances has different EndpointURL: cfg=%+v, eq=%+v", cfg, neq)
124+
}
110125
})
111126

112127
t.Run("GCS", func(t *testing.T) {
@@ -217,6 +232,59 @@ func TestIsSameStorage(t *testing.T) {
217232
t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
218233
}
219234
})
235+
236+
t.Run("oss", func(t *testing.T) {
237+
cfg := &oss.Config{
238+
Region: "eu",
239+
EndpointURL: "ep.com",
240+
Bucket: "b1",
241+
Prefix: "p1",
242+
Credentials: oss.Credentials{
243+
AccessKeyID: "k1",
244+
AccessKeySecret: "k2",
245+
SecurityToken: "sect",
246+
},
247+
ConnectTimeout: 10 * time.Second,
248+
UploadPartSize: 6 << 20,
249+
MaxObjSizeGB: floatPtr(1.1),
250+
Retryer: &oss.Retryer{},
251+
ServerSideEncryption: &oss.SSE{},
252+
}
253+
eq := &oss.Config{
254+
Region: "eu",
255+
EndpointURL: "ep.com",
256+
Bucket: "b1",
257+
Prefix: "p1",
258+
}
259+
if !cfg.IsSameStorage(eq) {
260+
t.Errorf("config storage should identify the same instance: cfg=%+v, eq=%+v, diff=%s",
261+
cfg, eq, cmp.Diff(*cfg, *eq))
262+
}
263+
264+
neq := cfg.Clone()
265+
neq.Region = "us"
266+
if cfg.IsSameStorage(neq) {
267+
t.Errorf("storage instances has different region: cfg=%+v, eq=%+v", cfg, neq)
268+
}
269+
270+
neq = cfg.Clone()
271+
neq.EndpointURL = "ep2.com"
272+
if cfg.IsSameStorage(neq) {
273+
t.Errorf("storage instances has different EndpointURL: cfg=%+v, eq=%+v", cfg, neq)
274+
}
275+
276+
neq = cfg.Clone()
277+
neq.Bucket = "b2"
278+
if cfg.IsSameStorage(neq) {
279+
t.Errorf("storage instances has different bucket: cfg=%+v, eq=%+v", cfg, neq)
280+
}
281+
282+
neq = cfg.Clone()
283+
neq.Prefix = "p2"
284+
if cfg.IsSameStorage(neq) {
285+
t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
286+
}
287+
})
220288
}
221289

222290
func TestCastError(t *testing.T) {

‎pbm/storage/azure/config.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ func (cfg *Config) IsSameStorage(other *Config) bool {
7070
if cfg.Prefix != other.Prefix {
7171
return false
7272
}
73+
if cfg.EndpointURL != other.EndpointURL {
74+
return false
75+
}
76+
if !maps.Equal(cfg.EndpointURLMap, other.EndpointURLMap) {
77+
return false
78+
}
79+
7380
return true
7481
}
7582

‎pbm/storage/oss/client.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ func (cfg *Config) IsSameStorage(other *Config) bool {
7878
if cfg.Prefix != other.Prefix {
7979
return false
8080
}
81+
if cfg.EndpointURL != other.EndpointURL {
82+
return false
83+
}
84+
8185
return true
8286
}
8387

‎pbm/storage/s3/s3.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,12 @@ func (cfg *Config) IsSameStorage(other *Config) bool {
196196
if cfg.Prefix != other.Prefix {
197197
return false
198198
}
199+
if cfg.EndpointURL != other.EndpointURL {
200+
return false
201+
}
202+
if !maps.Equal(cfg.EndpointURLMap, other.EndpointURLMap) {
203+
return false
204+
}
199205
return true
200206
}
201207

0 commit comments

Comments
 (0)