lifecycle stage refactor to ensure proper start and stop ordering of servers and announcements#7234
Conversation
gianm
left a comment
There was a problem hiding this comment.
Generally LG but I am confused about what's going on with the DerivativeDataSourceManager.
| * When optimize query, DerivativesManager offers the information about derivatives. | ||
| */ | ||
| @ManageLifecycleLast | ||
| @ManageLifecycleAnnouncements |
There was a problem hiding this comment.
Why is this in @ManageLifecycleAnnouncements? It doesn't seem to be doing announcements?
There was a problem hiding this comment.
Good question, I'll see if I can dig up why it was marked @ManageLifecycleLast prior to this PR...
There was a problem hiding this comment.
There is no reason in the initial PR, it was already in the first commit and not an explicit response to review afaict. Looking over what it is doing, it doesn't seem like it would need to be in the last stage, I will move it to just @ManageLifecycle
There was a problem hiding this comment.
Ok, moved it to Lifecycle.Stage.NORMAL, thanks for review!
gianm
left a comment
There was a problem hiding this comment.
I can't tell if this approach is clunky or elegant. Let's go with elegant :)
…servers and announcements (apache#7234) * lifecycle stage refactor to ensure proper ordering of servers and announcements * move DerivativeDataSourceManager to Lifecycle.Stage.NORMAL
…servers and announcements (apache#7234) * lifecycle stage refactor to ensure proper ordering of servers and announcements * move DerivativeDataSourceManager to Lifecycle.Stage.NORMAL
…servers and announcements (apache#7234) * lifecycle stage refactor to ensure proper ordering of servers and announcements * move DerivativeDataSourceManager to Lifecycle.Stage.NORMAL
This fixes an issue introduced by #7215.
HttpServerInventoryViewuses the 'discovery' announcement so would correctly identify servers that disappear, butAbstractCuratorServerInventoryViewwatches another announcement that stops later, causing the 'server disappeared' event to happen after jetty has stopped.This PR resolves this issue by slightly reworking the
Lifecycle.Stageenumeration and renaming them to be more relevant to their intended usage, to quote the javadocs:This has the very nice behavior that all announcements can be unannounced prior to stopping the server, meaning
druid.server.http.unannouncePropagationDelayshould apply to all announcements to give adequate time for the rest of the cluster to notice all announcements are gone.Logs of shutdown after this PR for curator based segment management:
and for http segment management: