node/ir: Handle double SIGHUP correctly #1197

elebedeva · 2024-06-24T12:20:39Z

elebedeva commented

2024-06-24 12:20:39 +00:00

Close #1145

Signed-off-by: Ekaterina Lebedeva ekaterina.lebedeva@yadro.com

Close #1145 Signed-off-by: Ekaterina Lebedeva <ekaterina.lebedeva@yadro.com>

elebedeva force-pushed fix/double-sighup from 331434e0ea to 92ee14bd89

2024-06-24 12:25:28 +00:00

Compare

elebedeva requested review from storage-core-committers 2024-06-24 12:42:35 +00:00

elebedeva requested review from storage-core-developers 2024-06-24 12:42:39 +00:00

fyrchik requested changes 2024-06-24 13:11:25 +00:00

cmd/frostfs-ir/config.go Outdated

					
				@ -76,3 +60,1 @@

								shutdown()

								log.Info(logs.FrostFSNodeTerminationSignalProcessingIsComplete)

								return

						case <-ch:

fyrchik commented

2024-06-24 13:11:23 +00:00

In your implementation, we now have a race condition between ch and sighupCh.
However, if we received SIGTERM there is no need to SIGHUP, because we will eventually shutdown.
I suggest make SIGTERM/SIGINT having priority over SIGHUP.

In your implementation, we now have a race condition between `ch` and `sighupCh`. However, if we received SIGTERM there is no need to SIGHUP, because we will eventually shutdown. I suggest make SIGTERM/SIGINT having priority over SIGHUP.

acid-ant commented

2024-06-25 06:14:47 +00:00

Also, there are is no guarantee that we receive all signals, even if we have buffered channel.

Also, there are is no [guarantee](https://github.com/golang/go/blob/b3b4556c245c8f21872910ee866133428bbb5a60/src/os/signal/signal.go#L109) that we receive all signals, even if we have buffered channel.

fyrchik commented

2024-06-25 06:42:30 +00:00

Also, there are is no guarantee

It was the problem of the old implementation. Now we have 2 separate channels and SIGTERM / SIGINT are handled similarly, so we should not lose anything useful.

>Also, there are is no guarantee It was the problem of the old implementation. Now we have 2 separate channels and SIGTERM / SIGINT are handled similarly, so we should not lose _anything useful_.

👍 1

elebedeva commented

2024-06-25 09:27:14 +00:00

fixed