node: Improve shutdown #930

acid-ant · 2024-01-25T06:30:45Z

acid-ant commented

2024-01-25 06:30:45 +00:00

Stop internal activity of the gc by context.
Release task pool of the policer when context cancelled.

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

Stop internal activity of the `gc` by context. Release task pool of the `policer` when context cancelled. Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>

acid-ant force-pushed bugfix/improve-shutdown from 4ecf408e19 to 748b173fc3

2024-01-25 06:32:42 +00:00

Compare

fyrchik requested review from storage-core-committers 2024-01-25 07:25:30 +00:00

fyrchik requested review from storage-core-developers 2024-01-25 07:25:30 +00:00

fyrchik reviewed 2024-01-25 07:41:57 +00:00

pkg/local_object_storage/engine/shards.go

					
				@ -339,1 +341,4 @@

							return

						default:

						}

						sh.NotificationChannel() <- ev

fyrchik commented

2024-01-25 07:27:57 +00:00

Shouldn't it be select with both this and <-ctx.Done() branches?
A separate select for ctx.Done() is ok, because this way we guarantee early exit, though a single select is not much worse.

Shouldn't it be `select` with both this and `<-ctx.Done()` branches? A separate `select` for `ctx.Done()` is ok, because this way we guarantee early exit, though a single select is not much worse.

acid-ant commented

2024-01-25 07:53:39 +00:00

Right, the idea was to exit earlier. And it looks like once NotificationChannel is buffered, we can use one select for this too statements.

Right, the idea was to exit earlier. And it looks like once `NotificationChannel` is buffered, we can use one `select` for this too statements.

pkg/local_object_storage/shard/control.go Outdated

					
				@ -151,0 +150,4 @@

						// Currently it is used only for listening for the new epoch event.

						// It is ok to keep in buffer one event, it helps us properly react on cancelling of the context.

						// In other case we need to keep the order of component shutdown, like closing morph before engine.

						eventChan: make(chan Event, 1),

fyrchik commented

2024-01-25 07:29:00 +00:00

Is it necessary if we implement select suggestion from another comment?

Is it necessary if we implement `select` suggestion from another comment?

acid-ant commented

2024-01-25 08:01:19 +00:00

Yes, because there might be situation when notification received and at the same time context cancelled. In scope of this PR, routine which reading from channel might end earlier than routine which write in it - this will lead to block on write in channel.

fyrchik commented

2024-01-26 10:14:55 +00:00

Hm, how exactly? If we have select with 2 branches and 1 is available (ctx.Done()), there is no way we will block

Hm, how exactly? If we have select with 2 branches and 1 is available (`ctx.Done()`), there is no way we will block

acid-ant commented

2024-01-26 10:57:34 +00:00

No more working from home, messed in your comments. Right, once select used for writing in eventChannel buffering here is redundant.

No more working from home, messed in your comments. Right, once `select` used for writing in `eventChannel` buffering here is redundant.

dstepanov-yadro commented

2024-01-25 07:52:35 +00:00

Does SIGHUP work correctly?

Does `SIGHUP` work correctly?

acid-ant force-pushed bugfix/improve-shutdown from 748b173fc3 to f01961bdf9

2024-01-25 08:03:30 +00:00

Compare

acid-ant commented

2024-01-25 08:22:24 +00:00

Does SIGHUP work correctly?

There are no changes in implementation for reaction on SIGHUP - for storage engine we are reconfiguring only shards.

> Does `SIGHUP` work correctly? There are no changes in implementation for reaction on SIGHUP - for `storage engine` we are reconfiguring only shards.

dstepanov-yadro approved these changes 2024-01-26 09:50:48 +00:00

acid-ant force-pushed bugfix/improve-shutdown from f01961bdf9 to ba7fa7eca1

2024-01-26 10:01:30 +00:00

Compare

acid-ant force-pushed bugfix/improve-shutdown from ba7fa7eca1 to 766ee97e81

2024-01-26 10:54:53 +00:00

Compare

acid-ant force-pushed bugfix/improve-shutdown from 766ee97e81 to 5f88bf9c77

2024-01-29 08:58:43 +00:00

Compare

acid-ant force-pushed bugfix/improve-shutdown from 5f88bf9c77 to 1f2c34318f

2024-01-30 07:14:12 +00:00