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

The notify implementation within nacos-common module radiates to all nacos modules #2859

Closed
chuntaojun opened this issue May 22, 2020 · 3 comments
Assignees
Milestone

Comments

@chuntaojun
Copy link
Collaborator

Issue Description

Type: feature request

Describe what happened (or what feature you want)

background

  1. At present, whether nacos is a client or a server, the code of the event mechanism is too fragmented, and the realization of a unified event mechanism is needed
    Ideas
  2. The code of the current event mechanism has sunk into the notification of nacos-common
  3. Replace the implementation of the notification of nacos-common with the code of the existing old event mechanism, and remove all EventDispatcher
  4. According to the nature of the event, determine whether a separate event publisher is needed

Describe what you expected to happen

How to reproduce it (as minimally and precisely as possible)

Tell us your environment

Anything else we need to know?

@chuntaojun chuntaojun added this to the 1.3.0 milestone May 22, 2020
@zongtanghu
Copy link
Collaborator

Hi chuntaojun @chuntaojun , this issue can be assign to me , thanks.

@KomachiSion KomachiSion modified the milestones: 1.3.0, 1.3.1 Jun 10, 2020
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jun 12, 2020
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jun 12, 2020
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jun 12, 2020
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jun 12, 2020
@zongtanghu
Copy link
Collaborator

zongtanghu commented Jun 19, 2020

"The notify implementation within nacos-common module radiates to all nacos modules" can be split into serveral pr, includes:

(1)Sink the Notify implementation into common module and optimize some parts.(PR Link is: #3118)
(2)Remove unit test case, fix typo and improve performance for NotifyCenter.(PR Link is: #3141 )
(2)Replace the original Notify implementation in the config module and which is called places in other module;(PR Link is: #3179 )
(3)Replace the original Notify implementation in the core/naming module;(PR's link is #3309);
(4)Replace Event Dispatcher in other module(The pr link is: #3319 and https://github.com/alibaba/nacos/pull/3313,https://github.com/alibaba/nacos/pull/3364);

@KomachiSion KomachiSion modified the milestones: 1.3.1, 1.3.2 Jun 28, 2020
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jul 13, 2020
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jul 13, 2020
KomachiSion pushed a commit that referenced this issue Jul 14, 2020
…le. (#3313)

* [ISSUE##2859]Replace some usage of EventDispatcher for ConfigCacheService and LongPollingService.

* [ISSUE##2859]Replace some usage of EventDispatcher for AsyncNotifyService and ConfigChangePublisher.

* [ISSUE#3179]fix typo.

* [ISSUE#3179]fix typo.

* [ISSUE#3179]fix typo.
zongtanghu added a commit to zongtanghu/nacos that referenced this issue Jul 14, 2020
@zongtanghu
Copy link
Collaborator

This issue has already been finished, except for naming event dispatcher.So I will close this issue, and reopen an other issue to follow up naming event dispatch replacement.

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

No branches or pull requests

3 participants