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

nacos注册中心重启后,AbstractServerHolderImpl保存的NamingService并没有更新 #243

Closed
zrlw opened this issue Jun 27, 2021 · 48 comments

Comments

@zrlw
Copy link
Contributor

zrlw commented Jun 27, 2021

AbstractServerHolderImpl里的serviceMap里的NamingService只在get方法获取时创建一次,没有更新机制,当nacos注册中心因故状态变成DOWN或者重启后,继续用这些保存的namingService会有问题。
如果不想大改,可以在AbstractServerHolderImpl增加一个remove(String clusterId, String namespace)方法,让继承AbstractServerHolderImpl的类自己判断get返回的namingService是否有效,无效则调用remove方法删除,再重新get获取。

@paderlol
Copy link
Contributor

因为重启或者down的状态移除掉,然后重新获取,这个连接重新获取就会正常吗?我可能没太理解?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

我们想让nacos sync(多实例,单库)同步多机房nacos注册中心(各自使用独立的数据库,每个注册中心3个节点),但是发现每个机房的注册实例数都不正确,大部分实例都是本机房注册的实例,其他机房同步过来的实例数量参差不齐。
我们在排查分析原因,因为期间有机房的注册中心的节点做维护状态做过DOWN或重启操作,看nacos sync代码发现只创建一次namingService,感觉有问题。

@paderlol
Copy link
Contributor

这个跟创建naming service只创建一次的关联在哪里?你server端都挂了,我这里移除连接单例,然后重新获取的意义是什么?重新获取就可以使用了吗?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

重新获取是指当注册中心恢复后重新获取,恢复之前一直告警即可。不是还有3秒定时任务么?定时任务每次都尝试重新获取啊

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

注册中心有多个节点,nacos sync创建的namingServie连接的是一个节点还是全部节点?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

如果server挂了,SkyWalkerCacheServices保存的finishedTaskMap也需要清理,否则QuerySyncTaskTimer也不会重新发出同步事件。

@paderlol
Copy link
Contributor

你如果配置的集群,就是集群所有连接

@paderlol
Copy link
Contributor

Sync利用的就是Nacos自己的客户端,你是eureka同步?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

我们的场景是服务实例频繁上下线,事件通知的先后时序可能出现混乱,但是我们在管理界面手工执行了sync同步任务之后,结果还是和同步操作前一个样子,所以我们想看看有没有可能用的是失效的namingService。
我们都是nacos,各个机房都有一套多节点的nacos注册中心。

@paderlol
Copy link
Contributor

如果全部是Nacos跟那个定时任务没有任何关系,那个地方只是开启而已,主要是依赖Nacos本身的事件机制,你看代码看错地方了吧

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

public class QuerySyncTaskTimer implements CommandLineRunner {
    @Autowired
    private MetricsManager metricsManager;

    @Autowired
    private SkyWalkerCacheServices skyWalkerCacheServices;

    @Autowired
    private TaskAccessService taskAccessService;

    @Autowired
    private EventBus eventBus;

    @Autowired
    private ScheduledExecutorService scheduledExecutorService;

    @Override
    public void run(String... args) {
        /** Fetch the task list from the database every 3 seconds */
        scheduledExecutorService.scheduleWithFixedDelay(new CheckRunningStatusThread(), 0, 3000,
                TimeUnit.MILLISECONDS);

    }

    private class CheckRunningStatusThread implements Runnable {

        @Override
        public void run() {

            Long start = System.currentTimeMillis();
            try {

                Iterable<TaskDO> taskDOS = taskAccessService.findAll();

                taskDOS.forEach(taskDO -> {

                    if ((null != skyWalkerCacheServices.getFinishedTask(taskDO))) {

                        return;
                    }
                    
                    if (TaskStatusEnum.SYNC.getCode().equals(taskDO.getTaskStatus())) {

                        eventBus.post(new SyncTaskEvent(taskDO));
                        log.info("从数据库中查询到一个同步任务,发出一个同步事件:" + taskDO);
                    }

这个不是3秒执行一次么?

@paderlol
Copy link
Contributor

这里针对Nacos的同步来讲只是开启操作而已,调用一次就不会调用了,你看到了判断代码吗?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

对,我们想改的就是利用这个判断,当server端down掉的时候,把skyWalkerCacheServices的FinishedTask清掉,让判断语句返回null,以便重新发起同步事件。

@paderlol
Copy link
Contributor

只有像Eureka或者Consul这种没事件机制的才会,像你刚才说的那种多sync实例,单库实际上是会跑多个重复的同步任务,还有如果你用的2.0以上版本,是没法多实例同步的,现在Nacos不支持,你们查看问题应该去观察事件接收的地方

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

server端down掉的时候,我们想把skyWalkerCacheServices里相关的FinishedTask全部清掉,然后shutdown掉namingService释放资源,等待QuerySyncTaskTimer重试,直至重试成功为止。
我们用的nacos 1.4.2

@paderlol
Copy link
Contributor

本质上挂掉了,事件应该也会一直在运行,为什么还要重试?你是想出现问题直接移除这个任务?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

事件虽然还在运行,但是注册中心恢复正常之后,貌似一直收不到事件通知了。

@paderlol
Copy link
Contributor

不对啊,客户端自己会重连的吧

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

如下所示,NacosSyncToNacosServiceImpl的sync方法调用了订阅,但是在注册中心恢复之后这个订阅貌似失效了。

NamingService sourceNamingService =
                nacosServerHolder.get(taskDO.getSourceClusterId(), taskDO.getGroupName());
...
sourceNamingService.subscribe(taskDO.getServiceName(), nacosListenerMap.get(taskDO.getTaskId()));

@paderlol
Copy link
Contributor

这个是服务端的bug吧,重连以后所有订阅失效了

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

nacos的文档上没有说是否保留原来的订阅

@paderlol
Copy link
Contributor

因为所有的rpc都是基于订阅来控制注册的啊

@paderlol
Copy link
Contributor

你们可以先基于你们自己的想法直接改了用,也可以自己改了提PR,订阅这个问题,我去确认下

@paderlol
Copy link
Contributor

不是Server保留订阅,而是Server推送事件,订阅是客户端自己保留在内存的,只要连接上有事件就会推送国泰

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

我先提了一个修复,可能和这个问题有关,也可能无关。
#244 修复的是当网络连接注册中心有问题时,启动nacos sync时不要将task任务加到SkyWalkerCacheServices的finishedTaskMap里,因为实现SyncService接口的各个同步服务类的sync和delete方法都捕获了异常,只是返回true或false,所以EventListener是捕获不到这些方法的异常的。

@paderlol
Copy link
Contributor

但是这个和你描述的问题似乎没关系,你描述得问题是运行过程中,server crashed之后,然后恢复接收不到事件了,但是Nacos一旦订阅之后逻辑就不会再次运行这个Sync方法了,而是在Nacos的事件里面

@paderlol
Copy link
Contributor

你修复这个仅仅只是让server不可用的时候无法开启任务而已

@zrlw
Copy link
Contributor Author

zrlw commented Jun 27, 2021

嗯,我们也不确定问题的根源,当时启动nacos sync的时候,没有确认nacos sync到所有机房nacos注册中心的网络是否已经全部打通,如果没打通就启动了nacos sync,我们认为是会触发这个现象的。
nacos注册中心运行过程中我们对有些节点做了下线又重新上线处理,这个操作会不会触发这个问题,我们还需要找个可以测试的环境做验证。

@paderlol
Copy link
Contributor

那先验证吧

@zrlw
Copy link
Contributor Author

zrlw commented Jun 28, 2021

我们基本上定位到问题原因了,应该是Sync 0.4.4同步了不该同步的实例所致。
过程如下:
1. A机房停止了服务实例S,A机房注册中心删除了服务实例S;
2. sync同步了B机房的S服务实例到A机房,A机房有带sourceClusterId标签的S服务实例;
3. 服务实例S马上重新启动了;
4. A机房注册中心收到S注册请求,但因为有了B机房同步过来的S,所以保留了同步过来的S;
5. B机房注册中心重启;
6. sync同步A机房到B,过滤掉了带有sourceClusterId标签的S服务实例。
诡异的地方是步骤2,看sync 0.4.4的SyncService接口的needSync是有判断sourceClusterId标签为空,但是我们现在在A机房看到了带有sourceClusterId标签=B机房的S服务实例,ip地址是A机房的,更诡异的是A、B两个机房存在这种情况的服务不止一种,所以应该是needSync在某种情况下失效所致。
对比了0.4.2和0.4.4的NacosSyncToNacosServiceImpl.java,发现sync方法获取sourceInstances从event.getInstances()改成了 sourceNamingService.getAllInstances.

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

我们的实例都是在nacos 1.4.2注册中心的public命名空间的DEFAULT_GROUP分组上注册的。我们的各个同步任务的分组字段都配成了public,并不是DEFALUT_GROUP,各个同步任务的命名空间都没有配置。
神奇的事情在于:
0.4.4版本的sync创建nacosNamingService用的是nacosServerHolder.get(taskDO.getSourceClusterId(), taskDO.getGroupName());, 而nacosServerHolder的get方法第二个入参是用来传命名空间的,所以我们配错的分组名称public刚好传对了。
0.4.4版本NacosSyncToNacosServiceImpl.java 获取服务实例列表时调用getAllInstances方法时只传了同步任务的服务名,没有传入分组:

List<Instance> sourceInstances = sourceNamingService.getAllInstances(taskDO.getServiceName(),
                            new ArrayList<>(), false);

这样,0.4.4使用的nacos client1.3.1会从默认的DEFAULT_GROUP分组去获取实例。
这样在0.4.4版本下,我们虽然配错了同步任务的分组名,但是sync还能跑起来,只是在机房灾备演练后会出现本帖所述的问题。

我们把sync升到0.4.5,发现同步的实例分组都变成了public分组,而不是DEFAULT_GROUP,对比了0.4.4和0.4.5差异才发现
0.4.5创建nacosNamingService时改成了nacosServerHolder.get(taskDO.getSourceClusterId(), taskDO.getNameSpace());
0.4.5同步任务时使用了同步任务配置的分组名,但是获取服务实例列表时并没有传入分组名。
也就是说无论0.4.4,还是0.4.5,都同步不了不是DEFAULT_GROUP分组的实例。

@paderlol
Copy link
Contributor

0.4.5 是支持分组的 具体可以看PR https://github.com/nacos-group/nacos-sync/pull/218/files 如果你看过源码就应该知道这里是接收了group的 所以我不知道你是怎么对比代码差异的

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

加了个NacosSyncToNacosServiceImpl的sync方法修复,在获取实例时传入了同步任务的分组名,以支持非DEFALUT_GROUP分组的同步任务。

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

0.4.5 是支持分组的 具体可以看PR https://github.com/nacos-group/nacos-sync/pull/218/files 如果你看过源码就应该知道这里是接收了group的 所以我不知道你是怎么对比代码差异的

public abstract class AbstractServerHolderImpl<T> implements Holder {
    ... ...
    public T get(String clusterId, String namespace) {
        final String finalNamespace = Optional.ofNullable(namespace).orElse(Strings.EMPTY);
         ...
                return createServer(clusterId, () -> skyWalkerCacheServices.getClusterConnectKey(clusterId),
                    finalNamespace);

我觉得可能你搞混了入参,createServer传入参数一直要求的是命名空间,并不是分组名。
0.4.4的sync方法写错成了nacosServerHolder.get(taskDO.getSourceClusterId(), taskDO.getGroupName()),0.4.5修复了而已。

@paderlol
Copy link
Contributor

组入参不是写错了,是以前sync认为组的纬度就是namespace,当时没有支持实例组这个纬度,你可以翻以前的issue就知道,然后你刚刚提交的确实是我的问题,再处理冲突的时候,粗心把其他贡献者的代码给覆盖了,本身这个PR是对的。

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

顺便提个建议,因为nacos 2.0+不支持单客户端同步多实例,所以最新的0.4.6可用性并不大,建议从0.4.5拉出一个分支,以便同步进行缺陷修订。

@paderlol
Copy link
Contributor

可以用

@paderlol
Copy link
Contributor

你不用2.0的客户端就行了

@paderlol
Copy link
Contributor

Pr需要通过develop分支提交develop,不要直接从master修改,提交master

@paderlol
Copy link
Contributor

我下午去更新一下develop分支,改回1.4.2的客户端,然后你再提供你的pr进来吧

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

重新提了基于develop分支的pull请求: #246
其中sync方法做了修订,只同步healthy状态为true的实例,我怀疑可能是实例healthy状态变成false时,nacos注册中心修改了元数据,删掉了nacos sync添加的sourceClusterId,导致sync同步出现了循环。

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

我看了一下nacos的InstanceController类,里面有3个地方会重置元数据,涉及register,update, beat和patch四个场景,很难保证nacos不会覆盖sync的标签。
nacos注册中心以客户端ip和端口为唯一,我觉得可以考虑大改一下,不用metadata,改用ConcurrentHashMap<SourceClusterID, List<Instance-IP-PORT>>,免得元数据被nacos中心干掉,只是各个同步任务需要增加一个IP列表字段了。
如果能和nacos社区沟通好,确保对方重置元数据之前保留sourceClusterId也可以。
还有一个方法就是sync自己保存一份快照,如果发现nacos注册中心删除了元数据,就重新打标搞回去。

@paderlol
Copy link
Contributor

那就没法防止ABC的问题了,A同步到B B把A的服务也同步到C

@paderlol
Copy link
Contributor

Nacos清除元数据的场景都是在实例或者服务失效,需要移除的时候

@zrlw
Copy link
Contributor Author

zrlw commented Jun 29, 2021

那就sync保留一份快照吧,在needSync判定方法里查一下自己保存的快照

@paderlol
Copy link
Contributor

你可以fork改动下,然后测试觉得可以,提个pr上来,然后看一下

@paderlol
Copy link
Contributor

但是我个人觉得你这么做会把事情搞的更加复杂,因为Nacos同步不是单向的,他更多是双向的,你得能区分哪些是自己的实例,哪些是同步过来的

@paderlol
Copy link
Contributor

我已经发了一个beta版本,合并你的问题,并且退回到了1.4.2的客户端. 有问题请新开issue

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

No branches or pull requests

2 participants