Skip to content

update internal-discovery Listener for node list and use same at router and coordinator#4697

Merged
cheddar merged 15 commits intoapache:masterfrom
himanshug:membership_pr1_2
Aug 25, 2017
Merged

update internal-discovery Listener for node list and use same at router and coordinator#4697
cheddar merged 15 commits intoapache:masterfrom
himanshug:membership_pr1_2

Conversation

@himanshug
Copy link
Copy Markdown
Contributor

@himanshug himanshug commented Aug 17, 2017

Follow-up to #4634

This patch

  • makes Coordinator discover lookup nodes using internal discovery and removes current zookeeper based ListenerDIscoverer
  • makes Router discover brokers using internal discovery instead of current zookeeper based "external discovery"
  • Updates DruidNodeDiscovery.Listener.nodesAdded(List nodes) to support "initialize" feature similar to provided by CuratorInventoryManager and is used in ServerInventoryView. HttpServerInventoryView is updated to utilize same. That is, first call to Listener.nodesAdded(list) means that underlying cache is initialized.

@cheddar
Copy link
Copy Markdown
Contributor

cheddar commented Aug 18, 2017

Could we switch the API to use lists of nodes instead of individual and have the contract be that the first initialization is actually given the full list?

@himanshug himanshug changed the title use internal-discovery at router and coordinator update internal-discovery Listener for node list and use same at router and coordinator Aug 21, 2017
@cheddar
Copy link
Copy Markdown
Contributor

cheddar commented Aug 24, 2017

👍

@cheddar cheddar merged commit 74538c3 into apache:master Aug 25, 2017
@himanshug himanshug deleted the membership_pr1_2 branch December 29, 2017 17:44
for (Listener listener : listeners) {
listener.nodeRemoved(node);
synchronized (lock) {
if (uninitializedNodeTypeListeners.isEmpty()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@himanshug could you explain what does this mean? The logic around uninitializedNodeTypeListeners is super convoluted. And it doesn't seem to work for Listeners that are not NodeTypeListener

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants