sdnotify: Fix status for RELOADING #1440

acid-ant · 2024-10-21T11:15:04Z

acid-ant commented

2024-10-21 11:15:04 +00:00

Before:

RELOADING=1
MONOTONIC_USEC=17951246687
STATUS=RELOADING=1
MONOTONIC_USEC=17951246687

After:

RELOADING=1
MONOTONIC_USEC=17951246687
STATUS=RELOADING

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Before: ``` RELOADING=1 MONOTONIC_USEC=17951246687 STATUS=RELOADING=1 MONOTONIC_USEC=17951246687 ``` After: ``` RELOADING=1 MONOTONIC_USEC=17951246687 STATUS=RELOADING ``` Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>

requested reviews from storage-core-committers, storage-core-developers

2024-10-21 11:15:20 +00:00

acid-ant force-pushed bugfix/sdnotify from 5be7aa3545 to 60911b7cb1

2024-10-21 11:15:37 +00:00

Compare

dstepanov-yadro approved these changes 2024-10-21 11:19:02 +00:00

fyrchik reviewed 2024-10-21 11:22:33 +00:00

pkg/util/sdnotify/sdnotify.go Outdated

					
				@ -59,6 +59,8 @@ func FlagAndStatus(status string) error {

							return fmt.Errorf("clock_gettime: %w", err)

						}

						status += "\nMONOTONIC_USEC=" + strconv.FormatInt(ts.Nano()/1000, 10)

						status += "\nSTATUS=RELOADING"

fyrchik commented

2024-10-21 11:22:33 +00:00

What is the difference with the previous implementation?

fyrchik approved these changes 2024-10-21 11:24:01 +00:00

acid-ant force-pushed bugfix/sdnotify from 60911b7cb1 to 8b6ec57c61

2024-10-21 11:25:37 +00:00

Compare

acid-ant commented

2024-10-21 11:25:50 +00:00

Commit message updated.

a-savchuk reviewed 2024-10-21 11:32:03 +00:00

pkg/util/sdnotify/sdnotify.go

					
				@ -59,6 +59,8 @@ func FlagAndStatus(status string) error {

							return fmt.Errorf("clock_gettime: %w", err)

						}

						status += "\nMONOTONIC_USEC=" + strconv.FormatInt(ts.Nano()/1000, 10)

						status += "\nSTATUS=RELOADING"

a-savchuk commented

2024-10-21 11:32:03 +00:00

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

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

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

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.

a-savchuk marked this conversation as resolved

elebedeva approved these changes 2024-10-21 11:54:04 +00:00

acid-ant merged commit 8b6ec57c61 into master

2024-10-21 11:55:59 +00:00

acid-ant deleted branch bugfix/sdnotify

2024-10-21 11:56:01 +00:00

acid-ant referenced this pull request from a commit

2024-10-21 11:56:01 +00:00