Skip to content
This repository has been archived by the owner on Aug 30, 2019. It is now read-only.

Graceful shutdown for Windows Service #330

Merged
merged 3 commits into from
Oct 30, 2017
Merged

Conversation

bmermet
Copy link

@bmermet bmermet commented Oct 30, 2017

This PR extracts the logic of providing an exit channel out of the runAgent function, that way we can have different exit conditions:

  • SIGINT or SIGTERM on unix
  • service STOP or SHUTDOWN when it is running as a windows service

@palazzem palazzem requested review from palazzem and talwai October 30, 2017 13:13
@palazzem palazzem added this to the 5.18.2 milestone Oct 30, 2017
@palazzem palazzem added the core label Oct 30, 2017
Copy link

@palazzem palazzem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What has been changed is clear to me. Let's wait for @talwai review and then we can merge and ship the new release. Thanks a lot!

elog.Error(1, fmt.Sprintf("unexpected control request #%d", c))
exit := make(chan struct{})

go func() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not have any implication. Something we're missing @talwai?

@bmermet bmermet force-pushed the bmermet/agentshutdown branch from 111dd7b to 475d0dc Compare October 30, 2017 13:27
close(exit)
return
default:
elog.Error(1, fmt.Sprintf("unexpected control request #%d", c))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What could these other command be? Are they all actual errors?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not error indeed, only signals that we don't handle. I'll add your comment in the bigger PR about making the trace-agent work properly as a Windows Service.

Copy link

@derekwbrown derekwbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go ahead and merge back to my branch. I tested on windows, and it starts/stops successfully.

@bmermet bmermet merged commit 0feeba7 into db/windows_apm Oct 30, 2017
@palazzem palazzem deleted the bmermet/agentshutdown branch October 30, 2017 17:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants