-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[ISSUE #3922] method createServiceIfAbsent in ServiceManager require sync #4713
Conversation
putServiceAndInit(service); | ||
if (!local) { | ||
addOrReplaceService(service); | ||
synchronized (this) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lock in this may affect the performance.
I think fix like following will be better
public void putService(Service service) {
if (!serviceMap.containsKey(service.getNamespaceId())) {
synchronized (putServiceLock) {
if (!serviceMap.containsKey(service.getNamespaceId())) {
serviceMap.put(service.getNamespaceId(), new ConcurrentHashMap<>(16));
}
}
}
*serviceMap.get(service.getNamespaceId()).putIfAbsent(service.getName(), service);
}
private void putServiceAndInit(Service service) throws NacosException {
putService(service);
*service = getService(service.getNamespaceId(), service.getName());
service.init();
consistencyService.listen(KeyBuilder.buildInstanceListKey(service.getNamespaceId(), service.getName(), true), service);
consistencyService.listen(KeyBuilder.buildInstanceListKey(service.getNamespaceId(), service.getName(), false), service);
Loggers.SRV_LOG.info("[NEW-SERVICE] {}", service.toJSON());
}
What's your think about
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, "synchronized (this)" in this method will affect the performance, your plan is better
* 'develop' of https://github.com/alibaba/nacos: Use SafeConstructor to parse yaml configuration for AbstractConfigChangeListener (alibaba#4753) [ISSUE alibaba#3922] method createServiceIfAbsent in ServiceManager require sync (alibaba#4713) fix cloning configuration last desc (alibaba#4697) [ISSUE alibaba#4661]configServletInner#doGetConfig code optimization (alibaba#4666) Use revision to set version and upgrade to 1.4.2-SNAPSHOT (alibaba#4711) Revert interceptor all ui problem Fix alibaba#4701 (alibaba#4703) delete comment. fix metadata batch operation may delete instance problem. Upgrade to 1.4.1 (alibaba#4695) fix alibaba#4686 (alibaba#4692) Build nacos console front for 1.4.1 (alibaba#4690)
…op-import-v1 * 'develop' of https://github.com/alibaba/nacos: Use SafeConstructor to parse yaml configuration for AbstractConfigChangeListener (alibaba#4753) [ISSUE alibaba#3922] method createServiceIfAbsent in ServiceManager require sync (alibaba#4713) fix cloning configuration last desc (alibaba#4697) [ISSUE alibaba#4661]configServletInner#doGetConfig code optimization (alibaba#4666) Use revision to set version and upgrade to 1.4.2-SNAPSHOT (alibaba#4711) Revert interceptor all ui problem Fix alibaba#4701 (alibaba#4703) delete comment. fix metadata batch operation may delete instance problem. Upgrade to 1.4.1 (alibaba#4695) fix alibaba#4686 (alibaba#4692) Build nacos console front for 1.4.1 (alibaba#4690) For checkstyle fix: fix Jraft WriteRequest type problem. Add server identity to replace user-agent white list. (alibaba#4683)
method createServiceIfAbsent in ServiceManager require sync #3922
What is the purpose of the change
change method createServiceIfAbsent sync
Brief changelog
Double-Check