Skip to content

Conversation

@ewencp
Copy link
Contributor

@ewencp ewencp commented Dec 30, 2014

The graceful shutdown period, during which new requests are rejected but
outstanding requests are allowed to finish, is configurable. A shutdown hook in
Application also allows additional resources not associated with any single
request to be cleaned up. In order to ensure the process does not exit before
the hook runs, lifecycle methods are added to Application that wrap and augment
the ones in Jetty's Server class.

The graceful shutdown period, during which new requests are rejected but
outstanding requests are allowed to finish, is configurable. A shutdown hook in
Application also allows additional resources not associated with any single
request to be cleaned up. In order to ensure the process does not exit before
the hook runs, lifecycle methods are added to Application that wrap and augment
the ones in Jetty's Server class.
@ewencp
Copy link
Contributor Author

ewencp commented Dec 30, 2014

@junrao probably makes sense for you to review this since you'll want to make sure it suits your needs for schema-registry. I have a version of rest-utils that cleans up some shared state on shutdown using this, so I think this should be good enough. See the example app for the modifications to main() that you need to make since Application now wraps the Jetty Server completely for start/stop.

@ewencp
Copy link
Contributor Author

ewencp commented Jan 7, 2015

@junrao Do you want to look over this to make sure it'll work for schema-registry before I merge? It has some other incidental changes that I need to address another issue, so I'd like to get it committed.

Copy link

Choose a reason for hiding this comment

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

Should we make this an abstract method to force the subclasses to define onShutdown()? Most apps probably have some resources that they need to clean up on shutdown.

Other than that, the patch looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered that since I agree most apps probably have some resources that should get cleaned up gracefully, but doing so doesn't scale down to tests and simple apps like the example. You end up having to implement methods just to leave them empty. I'm going to leave it as is for now but we can always revisit.

ewencp added a commit that referenced this pull request Jan 7, 2015
Add graceful shutdown and a shutdown hook in Application.
@ewencp ewencp merged commit c929875 into master Jan 7, 2015
@ewencp ewencp deleted the shutdown-hook branch January 7, 2015 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants