alibaba / nacos

an easy-to-use dynamic service discovery, configuration and service management platform for building cloud native applications.
https://nacos.io
Apache License 2.0
30.32k stars 12.85k forks source link

nacos-client ServiceInfoHolder cache serviceInfo bug #8699

Open hh13516117509 opened 2 years ago

hh13516117509 commented 2 years ago

When I am using the Nacos client and start namingloadcacheatstart If multiple local services are enabled, but the same Nacos is not connected, and there is a service with the same name, it will lead to reading services in other Nacos I think this problem is relatively easy to solve. In the initcachedir method,

    if (!StringUtils.isBlank(jmSnapshotPath)) {
        cacheDir = jmSnapshotPath + File.separator + FILE_PATH_NACOS + namingCacheRegistryDir
                + File.separator + FILE_PATH_NAMING + File.separator + namespace;
    } else {
        cacheDir = System.getProperty(USER_HOME_PROPERTY) + File.separator + FILE_PATH_NACOS + namingCacheRegistryDir
                + File.separator + FILE_PATH_NAMING + File.separator + namespace;
    }

change:

    String suffix = File.separator + FILE_PATH_NACOS +File.separator +
            properties.getProperty("serverAddr")
            + namingCacheRegistryDir + File.separator + FILE_PATH_NAMING + File.separator + namespace;
    if (!StringUtils.isBlank(jmSnapshotPath)) {
        cacheDir = jmSnapshotPath + suffix;
    } else {
        cacheDir = System.getProperty(USER_HOME_PROPERTY) + suffix;
    }

It can achieve the effect of isolation

KomachiSion commented 2 years ago

But this change will make the new and old client can't compatible

hh13516117509 commented 2 years ago

我觉得,旧版客户端存储的路径是 \nacos\naming\dev 新版客户端的存储路径是 \nacos\serverAddress\naming\dev 这并不冲突.如果升级客户端版本之后希望能读取到原本的本地存储文件,可以考虑手动复制文件. 如果想在代码中做兼容处理的话,那我觉得可以考虑在 读取本地文件的时候,先在新路径下读取,如果没有读取到,在从老路径下读取,然后保存在新路径下,这样只需要在升级后第一次启动的时候自动完成文件迁移即可. 你觉得呢?

hh13516117509 commented 2 years ago
public ServiceInfoHolder(String namespace, Properties properties) {
    initCacheDir(namespace, properties);
    if (isLoadCacheAtStart(properties)) {
        this.serviceInfoMap = new ConcurrentHashMap<String, ServiceInfo>(readLocalCache());
    } else {
        this.serviceInfoMap = new ConcurrentHashMap<String, ServiceInfo>(16);
    }
    this.failoverReactor = new FailoverReactor(this, cacheDir);
    this.pushEmptyProtection = isPushEmptyProtect(properties);
}

public Map<String, ServiceInfo> readLocalCache(){
    Map<String, ServiceInfo> cache = DiskCache.read(this.cacheDir);
    if (cache.isEmpty()){
        String oldDir =
                System.getProperty(USER_HOME_PROPERTY) + File.separator + FILE_PATH_NACOS + namingCacheRegistryDir
                + File.separator + FILE_PATH_NAMING + File.separator + namespace;
        cache = DiskCache.read(oldDir);
        for (ServiceInfo simpleCache : cache.values()) {
            DiskCache.write(simpleCache, this.cacheDir);
        }
    }
    return cache;
}
KomachiSion commented 2 years ago

我觉得,旧版客户端存储的路径是 \nacos\naming\dev 新版客户端的存储路径是 \nacos\serverAddress\naming\dev 这并不冲突.如果升级客户端版本之后希望能读取到原本的本地存储文件,可以考虑手动复制文件. 如果想在代码中做兼容处理的话,那我觉得可以考虑在 读取本地文件的时候,先在新路径下读取,如果没有读取到,在从老路径下读取,然后保存在新路径下,这样只需要在升级后第一次启动的时候自动完成文件迁移即可. 你觉得呢?

以前存放在一个目录下会有冲突,在更新版本的时候,如果采用移动逻辑会导致把多余的数据也移动到了新路径下,这部分数据仍然是脏数据。

从设计原则上,应该是要隔离的,我记得配置中心是做了隔离的。 这里主要是需要如果不兼容读取就缓存数据的风险,需要充分的说明。

我觉得可以这样改, 先读取新的隔离数据,未读取到再读取旧的数据,不做数据拷贝。新的数据变更时只写入到新的路径里。这样至少能保证平滑过度到新版本。 至于是否删除旧版本脏数据,就交给用户自己决定了。

hh13516117509 commented 2 years ago

那倒也是,照样会把脏数据读到新路径下,虽然会很快刷新然后刷盘,但是确实会有部分脏文件,虽然没啥大影响, 是的. 我比较同意你的说法.是个比较合适的方案.

KomachiSion commented 2 years ago

可以看下社区中还有没有更多的建议,如果没什么意见就可以开始修改了。

hh13516117509 commented 2 years ago

好的.我没啥其他建议了.

onewe commented 2 years ago

我觉得没必要去兼容旧数据, 没了就重新获取嘛, 不用搞得这么麻烦

CherishCai commented 2 years ago

我觉得没必要去兼容旧数据, 没了就重新获取嘛, 不用搞得这么麻烦

+1,只要新版客户端获取到数据,落入到新的路径下就行啦,旧数据可以不管它。只要新版成功用上这件事就过去了,如果获取失败,用户应该回滚旧包那么就是旧客户端逻辑。

hh13516117509 commented 2 years ago

如果为了客户无感知的升级.可以考虑兼容旧数据,鲁棒性更高点. 如果为了方便快捷,代码简洁,那不兼容旧数据,只保存到新路径即可. 两种方式各有利弊吧.

KomachiSion commented 2 years ago

我看了下,启动时从磁盘中优先读取数据的功能默认开关是关的,需要通过开关打开,那直接更换的话,好像影响也不是很大,当然能实现兼容老版本的逻辑自然更好。可以保持几个版本,之后再删除。

但是这里需要在文档中写清楚。

KomachiSion commented 2 years ago

我觉得适配一下还是需要的, 新进展是dubbo会默认打开nacos的这个功能,用户不传这个参数的时候,dubbo会自动传入。否则可能影响dubbo启动时的寻址。

hh13516117509 commented 2 years ago

可以考虑先兼容几个版本的.在文档中以及源码中标记好这块属于兼容性代码,后续版本会进行一个清理即可.在合适的版本里再将这块代码进行一个清理.

KomachiSion commented 2 years ago

可以考虑先兼容几个版本的.在文档中以及源码中标记好这块属于兼容性代码,后续版本会进行一个清理即可.在合适的版本里再将这块代码进行一个清理.

可以,欢迎提交PR

stale[bot] commented 1 year ago

Thanks for your feedback and contribution. But the issue/pull request has not had recent activity more than 180 days. This issue/pull request will be closed if no further activity occurs 7 days later. We may solve this issue in new version. So can you upgrade to newest version and retry? If there are still issues or want to contribute again. Please create new issue or pull request again.