TrueCloudLab/frostfs-node
19
1
Fork 27
Code Issues 91 Pull requests 6 Projects Releases 1 Activity Actions

sdnotify: Fix status for RELOADING #1440

Merged
acid-ant merged 1 commit from acid-ant/frostfs-node:bugfix/sdnotify into master 2024-10-21 11:55:59 +00:00
Conversation 1 Commits 1 Files changed 1 +2
1 changed files with 2 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
pkg/util/sdnotify/sdnotify.go
View file

@ -59,6 +59,8 @@ func FlagAndStatus(status string) error {
return fmt.Errorf("clock_gettime: %w", err) return fmt.Errorf("clock_gettime: %w", err)
} }
status += "\nMONOTONIC_USEC=" + strconv.FormatInt(ts.Nano()/1000, 10) status += "\nMONOTONIC_USEC=" + strconv.FormatInt(ts.Nano()/1000, 10)
status += "\nSTATUS=RELOADING"
a-savchuk marked this conversation as resolved
a-savchuk commented 2024-10-21 11:32:03 +00:00
Review
Copy link

Why not use this approach for other statuses as well? like READY=1 and STOPPING=1

Why not use this approach for other statuses as well? like READY=1 and STOPPING=1
elebedeva commented 2024-10-21 11:40:30 +00:00
Review
Copy link

Why using this approach in the first place? We already adding this line to status here

Why using this approach in the first place? We already adding this line to `status` [here](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/ed13387c0e1406040898fc91f36c6d9370f58413/pkg/util/sdnotify/sdnotify.go#L63)
elebedeva commented 2024-10-21 11:53:29 +00:00
Review
Copy link

Ah, I see, we added MONOTONIC_USEC to status, so just trimming suffix =1 doesn't work anymore in case of reloading.

But I still believe that for other statuses changing current behaviour is unnecessary.

Ah, I see, we added `MONOTONIC_USEC` to `status`, so just trimming suffix `=1` doesn't work anymore in case of reloading. But I still believe that for other statuses changing current behaviour is unnecessary.
fyrchik commented 2024-10-21 12:26:16 +00:00
Review
Copy link

This line is inside if status == ReloadingEnabled, so only reloading is affected.
I think this answers both questions.

This line is inside `if status == ReloadingEnabled`, so only reloading is affected. I think this answers both questions.
return Send(status)
} }
status += "\nSTATUS=" + strings.TrimSuffix(status, "=1") status += "\nSTATUS=" + strings.TrimSuffix(status, "=1")
return Send(status) return Send(status)