ir: Change log level on SIGHUP #258
No reviewers
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
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
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#258
Loading…
Reference in a new issue
No description provided.
Delete branch "acid-ant/frostfs-node:feature/125-ir-sighup-log"
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?
Related to #125
Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com
How is it related to #125?
We already have functionality to reload config but all of it located in
cmd-node
. This is the first part of refactoring - to set new log level on SIGHUP we need to reload config. At next steps of refactoring node I'm planning to moveclosers
,workers
andhttpcomponent
fromnode
tointernal/common/config
. This will help to reloadpprof
andprometheus
I would like to choose a lighter approach in IR, all this package-per-config-section packages in node are really heavy and an overkill for IR.
No, suggested to use only functions which related to config reload. Agree that package-per-config-section is a nightmare for IR and planning to use direct
viper
. For other components (httpcomponent
,workers
,closers
) I think it is ok to reuse code from node.Also found that currently in IR we don't wait for closing of pprof and prometheus.
node: Moveto WIP: node: Moveconfig
from node tointernal/common
config
from node tointernal/common
After discussion with @fyrchik had a decision to refactor
cmd-node
only in part of config reloading.91230f36d4
to5193769165
5193769165
to57be7afbf0
WIP: node: Moveto ir: Change log level on SIGHUPconfig
from node tointernal/common
57be7afbf0
to3efc1e32e1
@ -48,3 +37,1 @@
if o.path != "" {
v.SetConfigFile(o.path)
func New(_ Prm, opts ...configViper.Option) *Config {
Remove
_ Prm
from parameters?Removed it totally.
@ -0,0 +48,4 @@
for {
select {
case <-ctx.Done():
hmm. if ctx cancelled to it will be infinite loop.
Thanks, removed this, refactored a bit main and config.
3efc1e32e1
to0760a7a6b2
0760a7a6b2
to145c2b2744
@ -0,0 +48,4 @@
for {
select {
case err := <-intErr:
{
What value does this extra block have?
Removed redundant brackets.
@ -144,2 +136,3 @@
}
return httpServers
//nolint:contextcheck
Let's avoid
nolint
in new code.No more necessary to use
//nolint:contextcheck
here.@ -35,7 +36,9 @@ func TestConfigEnv(t *testing.T) {
value = "some value"
)
err := os.Setenv(internal.Env(section, name), value)
Why the commit tells us it was
unused
?Also, could use
node:
prefix in the commit message.Fixed.
@ -0,0 +3,4 @@
import "github.com/spf13/viper"
type Opts struct {
Path string
Why are the fields public? In this case there is less need in functional options pattern (
With*
).Agree, refactored and changed to private, please review.
145c2b2744
toa2467f808e
a2467f808e
to54ce01225e
54ce01225e
to25d67cff56
25d67cff56
to14f83b8aa9