Routing object lifecycle management #418
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RoutingObjectHandler: Trigger lifecycle method (stop) for a removed or replaced routing object.
mikkokar
force-pushed
the
lifecycle-handlers
branch
from
71ba3ba
to
3c24a06
Compare
mikkokar
requested review from
kvosper,
VivianLopes and
dvlato
and removed request for
kvosper and
VivianLopes
kvosper
suggested changes
May 28, 2019
| @@ -58,6 +58,7 @@ | |||
| private static final String INTERCEPTOR_PIPELINE = "InterceptorPipeline"; | |||
| private static final String PROXY_TO_BACKEND = "ProxyToBackend"; | |||
| private static final String PATH_PREFIX_ROUTER = "PathPrefixRouter"; | |||
| private static final String HOST_PROXY = "HostProxy"; | |||
There was a problem hiding this comment.
It looks like this new field is not used?
There was a problem hiding this comment.
Oh. left there in when moving host proxy related code out. I'll remove this.
| RequestTracker tracker = trackRequests ? CurrentRequestTracker.INSTANCE : RequestTracker.NO_OP; | ||
| this.handler = new StandardHttpPipeline(interceptors, handler, tracker); | ||
| this.handler = handler; |
There was a problem hiding this comment.
Suggested change
| this.handler = handler; | |
| this.handler = requireNonNull(handler); |
|
|
||
| @Override | ||
| public CompletableFuture<Void> stop() { | ||
| pathPrefixRouter.routes.forEach((route, value) -> value.stop()); |
There was a problem hiding this comment.
Suggested change
| pathPrefixRouter.routes.forEach((route, value) -> value.stop()); | |
| pathPrefixRouter.routes.forEach((route, routingObject) -> routingObject.stop()); |
| @@ -30,7 +30,10 @@ | |||
| * Resolves a routing object reference from route database. | |||
| */ | |||
| public interface RouteRefLookup { | |||
| HttpHandler apply(RoutingObjectReference route); | |||
| // Consider modifying this interface to return Optional<RoutingObject>. | |||
There was a problem hiding this comment.
Is this comment still relevant?
There was a problem hiding this comment.
Yeah. It was originally a TODO, but then decided to leave it as a normal comment. I'm bit indecisive at the moment.
kvosper
approved these changes
May 31, 2019
dvlato
pushed a commit
that referenced
this pull request
Jul 3, 2019
* StyxObjectStore: insert/remove returns any replaced or removed value. * RoutingObjectHandler: Trigger lifecycle method (stop) for a removed or replaced routing object.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Lifecycle management is needed for cleaning up stateful resources that routing objects may have.
This is necessary for a new
HostProxyobject (under development) for proxying traffic to an individual remote host. It maintains a connection pool which needs shutting after the object is removed.This PR introduces a new
RoutingObjectinterface that extends anHttpHandler. It adds astoplifecycle method. Each routing object implements aRoutingObjectand thestopalong with it. The default implementation forstoplifecycle method is to do nothing.