Skip to content

Commit

Permalink
Stop notification for signals before, increase channel size. (#6522)
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu authored Nov 23, 2022
1 parent 5b004ba commit 6917e29
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
11 changes: 11 additions & 0 deletions .chloggen/fixchannel.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: service

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Stop notification for signals before shutdown, increase channel size.

# One or more tracking issues or pull requests related to the change
issues: [6522]
23 changes: 11 additions & 12 deletions service/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,12 @@ func New(set CollectorSettings) (*Collector, error) {
}

return &Collector{
set: set,
state: atomic.NewInt32(int32(StateStarting)),
shutdownChan: make(chan struct{}),
signalsChannel: make(chan os.Signal, 1),
set: set,
state: atomic.NewInt32(int32(StateStarting)),
shutdownChan: make(chan struct{}),
// Per signal.Notify documentation, a size of the channel equaled with
// the number of signals getting notified on is recommended.
signalsChannel: make(chan os.Signal, 3),
asyncErrorChannel: make(chan error),
}, nil
}
Expand Down Expand Up @@ -183,6 +185,7 @@ func (col *Collector) Run(ctx context.Context) error {

// Always notify with SIGHUP for configuration reloading.
signal.Notify(col.signalsChannel, syscall.SIGHUP)
defer signal.Stop(col.signalsChannel)

// Only notify with SIGTERM and SIGINT if graceful shutdown is enabled.
if !col.set.DisableGracefulShutdown {
Expand All @@ -197,7 +200,6 @@ LOOP:
col.service.telemetrySettings.Logger.Error("Config watch failed", zap.Error(err))
break LOOP
}

if err = col.reloadConfiguration(ctx); err != nil {
return err
}
Expand All @@ -206,20 +208,17 @@ LOOP:
break LOOP
case s := <-col.signalsChannel:
col.service.telemetrySettings.Logger.Info("Received signal from OS", zap.String("signal", s.String()))
switch s {
case syscall.SIGHUP:
if err := col.reloadConfiguration(ctx); err != nil {
return err
}
default:
if s != syscall.SIGHUP {
break LOOP
}
if err := col.reloadConfiguration(ctx); err != nil {
return err
}
case <-col.shutdownChan:
col.service.telemetrySettings.Logger.Info("Received shutdown request")
break LOOP
case <-ctx.Done():
col.service.telemetrySettings.Logger.Info("Context done, terminating process", zap.Error(ctx.Err()))

// Call shutdown with background context as the passed in context has been canceled
return col.shutdown(context.Background())
}
Expand Down

0 comments on commit 6917e29

Please sign in to comment.