Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support service change event #2733

Closed
wants to merge 2 commits into from

Conversation

maketubo
Copy link

@maketubo maketubo commented May 4, 2020

#2133 What is the purpose of the change

to give a api to listen special event like (service remove, service register or just service info change).

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

1. 服务变化信息其实是通过pushReceiver接收并调用注入的hostReactor解析json信息并触发对应的变化分析逻辑
2. 此处加入分析后事件发布逻辑, 实质上是调用客户端自定义实现的监听逻辑,此逻辑不应耗时过长,否则将导致ack响应延迟。
3. 增加EventInvoker之后 无论是基于spring boot或者是spring cloud模式都可以自定义一个实现类针对不同的事件类型实现对应的业务逻辑, 建议使用spring Application event机制进行定制。
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@maketubo
Copy link
Author

maketubo commented May 4, 2020

if the PR be accept, for Nacos Spring Cloud Projects, we can add a eventpublisher and a evenlister as this simple spring event listener example, which u can very free to design for your client project story.

public class NacosInstanceCanceledEvent extends ApplicationEvent {

    GlobalEventType type;

    ServiceInfo oldInfo;

    ServiceInfo newInfo;

    public NacosInstanceCanceledEvent(Object source, GlobalEventType type, ServiceInfo oldInfo, ServiceInfo newInfo) {
        super(source);
        this.type = type;
        this.oldInfo = oldInfo;
        this.newInfo = newInfo;
    }

    //getter and setter
}
@Component
public class SpringEventPublisher extends EventPublisher implements ApplicationContextAware {


    ApplicationContext applicationContext;

    @Override
    public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
        this.applicationContext = applicationContext;
    }

    @Override
    public void publish(GlobalEventType type, ServiceInfo oldInfo, ServiceInfo newInfo) {
        applicationContext.publishEvent(new NacosInstanceCanceledEvent(this, type, oldInfo, newInfo));
    }
}
@Component
public class NacosStateChangeListener {

    @EventListener
    public void listen(NacosInstanceCanceledEvent event) {
        System.out.println(event.getType() + "\t" + event.getOldInfo() + " 服务下线");
    }
}

what we need is just to find a agent way to set your EventPublisher to HostReactor.
may override NacosServiceRegistry constructor like this

public NacosServiceRegistry(NacosDiscoveryProperties nacosDiscoveryProperties,
								EventPublisher springEventPublisher) {
		this.nacosDiscoveryProperties = nacosDiscoveryProperties;
		this.namingService = nacosDiscoveryProperties.namingServiceInstance();
		((NacosNamingService)namingService).getHostReactor().setEventPublisher(springEventPublisher);
	}

@chuntaojun
Copy link
Collaborator

You just need to add an Event and reuse the EventDispatcher, it doesn't have to be that complicated

@maketubo
Copy link
Author

maketubo commented May 5, 2020

Do you mean to listen to a namingEvent as this example?

NamingService namingService = NacosFactory.createNamingService("127.0.0.1:8848");
            namingService.subscribe("xxxServiceName", ... ,event->{
                System.out.println("i catch the event!");
            });

i know that EventDispatcher will pushlish namingEvent for changeService, but in issue 2133 what they need is a global event which not need to set a special service and other params.

if (!CollectionUtils.isEmpty(listeners)) {
for (EventListener listener : listeners) {
List<Instance> hosts = Collections.unmodifiableList(serviceInfo.getHosts());
listener.onEvent(new NamingEvent(serviceInfo.getName(), serviceInfo.getGroupName(), serviceInfo.getClusters(), hosts));
}
}

maybe something like zookeeper watcher. This is a zookeeper watcher example (随手百度第一个).

//连接启动k
        try {
            ZooKeeper zk = new ZooKeeper("10.218.137.73:2181", 500000,new Watcher() {
                   // 监控所有被触发的事件
                     public void process(WatchedEvent event) {
                     System.out.println("changing...");
                   }
              });
        //设置监听器 
            Watcher wc = new Watcher() {

                public void process(WatchedEvent event) {
                    // TODO Auto-generated method stub
                    if (event.getType() == EventType.NodeDataChanged) {  
                        System.out.println("change"); 
                    }  
                    if (event.getType() == EventType.NodeDeleted){  
                        System.out.println("dele");  
                    }  
                    if(event.getType()== EventType.NodeCreated){ 
                        System.out.println("create"); 
                }
                }

            };
            //进行轮询,其中exists方法用来询问状态,并且设置了监听器,如果发生变化,则会回调监听器里的方法。
            while(true)
            {
                zk.exists("/jianghuiwen", wc);
            }



        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (KeeperException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }



    }

This PR is a normal design like other Service Registry and their client.
https:/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/Watcher.java
https:/apache/curator/blob/master/curator-framework/src/main/java/org/apache/curator/framework/api/CuratorEventType.java
(但是这个PR应该还是有点简单,没有考虑太多细节,期待您@chuntaojun 给出新的建议。也许直接改一下namingEvent就好了;;

@chuntaojun
Copy link
Collaborator

参考这个pr:https:/alibaba/nacos/pull/2391/files

@maketubo
Copy link
Author

maketubo commented May 5, 2020

image
@chuntaojun 这个是issue作者的需求,其实我觉得这种业务不该由某个客户端来做,也许他们有自己的考虑吧。其实在zookeeper和nacos这样的CP>AP的注册中心才会考虑这样的客户端订阅事件。对于eureka这种AP>CP可能仅仅只是提供一个cacheRreshEvent 见此
你的意思是让他们还是指定一个servicename去调用还是提一个PR修改subscribe的入参提供一个全局的监听入口,实质上是修改pushlishEvent的逻辑,还是说建议他们放弃这个需求?

@maketubo
Copy link
Author

maketubo commented May 5, 2020

他们现在应该是基于许长老的使用Nacos实现Spring Cloud Gateway的动态路由| 去做的

@chuntaojun
Copy link
Collaborator

可以说在NamingService添加一个方法,subscribeGlobalService,重新创建一个事件类型就好了,不必要添加一个EventPublisher,并且在nacos-client有一些东西不要暴露出去,比如HostReactor,

@maketubo
Copy link
Author

maketubo commented May 5, 2020

我本来的想法是如此,但是后来想把接口暴露出去,让他们自己针对自己的业务实现一套发布和监听逻辑会不会更好。可以基于spring event去做(好管理,容易适配),又可以自己定义一个机制,基于队列和生成者消费者模型去做,publish的时候只是把事件放到队列中然后异步返回,有一个listenContainer来处理这个队列里的事件,这样的自由度就很高了,完全基于业务定制,而不是像现在直接去invoke onEvent,也许这个需求并没有复杂到这个程度。

@maketubo
Copy link
Author

maketubo commented May 5, 2020

目前PR中的方式确实不够优雅,而且setEventPublisher是在构造函数即init逻辑设置之后进行的,严格来讲会有问题。

@chuntaojun
Copy link
Collaborator

chuntaojun commented May 5, 2020 via email

@maketubo
Copy link
Author

maketubo commented May 5, 2020

@chuntaojun 很遗憾你没懂我的意思

@chuntaojun
Copy link
Collaborator

chuntaojun commented May 5, 2020 via email

@chuntaojun
Copy link
Collaborator

至少对于目前的实现来说,我觉得并不好,期待你的重新提交

@maketubo
Copy link
Author

maketubo commented May 5, 2020

我想我明白你的意思了,可能是表述和理解上的差异。我的事件切入点在PushReceiver收到消息的时候,
image
而你的事件切入点在监控changeService队列的EventDispatcher上。你需要的是一个是整合两者或者说把globalEvent就放在监控changeService队列的逻辑下面的实现。
目前需要找到一个方式拿到GlobalNamingEvent(假设有)的listener 然后针对不同的事件类型进行触发,修改的尴尬点在于目前listener列表是根据serviceInfo拿到的。你有什么建议么@chuntaojun

@maketubo
Copy link
Author

maketubo commented May 5, 2020

你的意思应该是以下的伪代码,应该要在EventDispatcher维护一个以GlobalEventType为key的listener列表就可以。

namingService.subscribeGlobal(EventType.APP_UP, new EventListener() {
            @Override
            public void onEvent(Event event) {
                System.out.println(((NamingEvent)event).getServiceName() + "服务上线");
            }
        });

@maketubo maketubo closed this May 5, 2020
@chuntaojun
Copy link
Collaborator

你的意思应该是以下的伪代码,应该要在EventDispatcher维护一个以GlobalEventType为key的listener列表就可以。

namingService.subscribeGlobal(EventType.APP_UP, new EventListener() {
            @Override
            public void onEvent(Event event) {
                System.out.println(((NamingEvent)event).getServiceName() + "服务上线");
            }
        });

关于怎么触发各种类型的回调,可以看看这个

public abstract class TestEventListener implements EventListener {

	@Override
	public final void onEvent(Event event) {
		if (event instanceof GlobalEvent) {
			GlobalEvent globalEvent = (GlobalEvent) event;
			int type = globalEvent.getType();
			switch (type) {
			case 1:
				onServiceRegister(globalEvent);
				break;
			case 2:
				onServiceDown(globalEvent);
				break;
			case 3:
				onServiceModify(globalEvent);
				break;
			case 4:
				onServiceNormal(globalEvent);
				break;
			default:
				throw new IllegalArgumentException();
			}
		}
	}

	protected void onServiceRegister(GlobalEvent event) {

	}

	protected void onServiceDown(GlobalEvent event) {

	}

	protected void onServiceModify(GlobalEvent event) {

	}

	protected void onServiceNormal(GlobalEvent event) {

	}

}

@maketubo
Copy link
Author

maketubo commented May 5, 2020

关于可能的改动点,可能如下

public class EventDispatcher {

    private ExecutorService executor = null;

    private BlockingQueue<ServiceInfo> changedServices = new LinkedBlockingQueue<ServiceInfo>();

    //private BlockingQueue<NamingEvent> globalEvents = new LinkedBlockingQueue<NamingEvent>();

    private ConcurrentMap<String, List<EventListener>> observerMap
        = new ConcurrentHashMap<String, List<EventListener>>();

    private ConcurrentMap<GlobalEventType, List<EventListener>> observerGlobalMap
        = new ConcurrentHashMap<GlobalEventType, List<EventListener>>();

   //  addGlobalListener() {
   //  to add a globalEventListener to observerGlobalMap
   //}
   ....
  NamingService增加一个接口
    void subscribeGlobal(GlobalEventType type, EventListener listener) throws NacosException;
实现类增加实现
@Override
    public void subscribeGlobal(String type, EventListener listener) throws NacosException {
        eventDispatcher.addGlobalListener(type, listener);
    }

接下来就是在 Notifier 遍历监听GlobalEvent的listener列表触发他们的onEvent了。

@chuntaojun 看到你提供的那个PR上的改动 觉得等那个PR的提交合到基线版本上再进行这个issue的处理也不迟。 其实我也想了一下,我的PR其实方向错了,其实我的意思,应该是给EventDispatcher抽象成一个接口,框架内部提供的只是默认的实现,外部可以重定义这个EventDispatcher,实现自己的发布逻辑,而无需再去定义一个eventPublisher。
但是毕竟我们在实际使用的过程中,自己去构建namingServie太少了,很多人都不知道springCloud项目中,EnableDiscoveryClient => NacosDiscoveryAutoConfiguration => 触发namingService的构建。而且本质上来说可能服务的上下线频率并不高,这并非一个需要特别设计和考虑性能的场景。我的PR显得鸡肋了,基于以上原因我关闭这个PR。

@yanlinly yanlinly changed the title Feature2fix2133 Support service change event May 14, 2020
@yanlinly yanlinly added this to the 1.3.0 milestone May 14, 2020
@yanlinly yanlinly mentioned this pull request May 15, 2020
5 tasks
@yanlinly yanlinly mentioned this pull request Jun 5, 2020
5 tasks
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.

4 participants