app: Refactor the default copies number setting #109
Labels
No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-s3-gw#109
Loading…
Reference in a new issue
No description provided.
Delete branch "ironbee/frostfs-s3-gw:refactor_set-copies-number"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Signed-off-by: Artem Tataurov a.tataurov@yadro.com
Closes: #101
[#101] app: Refactor the default copies number settingto app: Refactor the default copies number setting@ -155,0 +160,4 @@
parsedValue, err := strconv.ParseUint(unparsed[i], 10, 32)
if err != nil {
l.Error("cannot parse default copies numbers", zap.Error(err))
break
What you think about returning empty slice (i.e. default) in case of an error during parsing? I think this is more appropriate than having half parsed slice.
@ -642,0 +637,4 @@
Policy: a.settings.policies,
DefaultMaxAge: handler.DefaultMaxAge,
NotificatorEnabled: a.cfg.GetBool(cfgEnableNATS),
DefaultCopiesNumbers: []uint32{0},
Let's use
[]uint32{handler.DefaultCopiesNumber}
or drophandler.DefaultCopiesNumber
.Probably we can skip initialization of this field because empty/nil vector leads to default behavior (the same as vectore with one element
0
)Please, fix linter error:
f4f2c716f3
to744ae8396d
744ae8396d
toa048c3d811
Since this PR is about refactoring. Can we move this check to line 162? It's minor, but still
a048c3d811
toe57795bb99
Should we use here this
?
@ -659,1 +657,4 @@
if val := fetchDefaultCopiesNumbers(a.log, a.cfg); len(val) > 0 {
cfg.DefaultCopiesNumbers = val
}
a.log.Debug("setting default copies numbers", zap.Uint32s("vector", cfg.DefaultCopiesNumbers))
Let's change
Debug
toInfo
Why shouldn't it be the
debug
?Info
will be more consistent with other similar cases. And it will be more convenient to see this information about copies number without any changes on cluster (where logging level isinfo
)The same for #109 (comment)
@ironbee @alexvanin I forgot, should this PR reload
frostfs.set_copies_number
andplacement_policy.copies_numbers
by SIGHUP?No, see #104
e57795bb99
toe24bc3f2ce
All remaining log issues going to be fixed in #111