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

Improve socketchannel #1146

Merged
merged 1 commit into from
Jan 19, 2020
Merged

Improve socketchannel #1146

merged 1 commit into from
Jan 19, 2020

Conversation

cloudwu
Copy link
Owner

@cloudwu cloudwu commented Jan 10, 2020

See issue #1145

改进 socket channel, 在 auth 失败后,会尝试连接 backup 列表(若提供)中下一个 host 。

之前的实现 socket channel 仅当连接主 host 失败后才尝试连接 backup 。

由于 socket channel 实现了这个特性, mongodb driver 就不必额外实现 pickserver 功能。在找不到主节点时,直接抛出 error 即可。

这个 patch 的主要改变是将原来的 connect_once 函数包装了一层,原来逻辑放在内部函数 _connect_once 中,包装层会复制一份 backup 列表,在 _connect_once 建立连接失败(包括 socket 建立失败,及 auth 过程发生 error )后,尝试列表中没有试过的 host 。

注: 如果 auth 过程主动调用 changehost ,而非抛出 error ,则会重试整个 connect_once 过程。

btw,这个 patch 改动并不太大。之所以 diff 看起来很复杂,是因为 _connect_once 相对 connect_once 改动了缩进层次。

@xjdrew
Copy link
Contributor

xjdrew commented Jan 10, 2020

实现逻辑没什么意见。

担心的是维护backup实现在socketchannel里面,使其他使用socketchannel的库也受到了影响,而这个影响未必是想要的。可以考虑把_add_backup相关的逻辑挪到mongo?

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 10, 2020

backup 目前只有 mongo 用,一开始也是为 mongo 设计的。

这么做倒不是为了照顾 mongo ,是想明确一下 auth 过程是不是 socketchannel 建立连接的一部分。如果是,那么 auth 失败后,我认为应该视为连接建立不成功,应该去尝试 backup 。

目前如果其它 socketchannel 用户若受影响,应该满足几个条件:

  1. 使用了 backup 机制,配置多个 host
  2. 使用了 auth 机制
  3. auth 有可能出错,希望 auth 出错后不尝试 backup

_add_backup 那段针对的是 auth 过程中 changebackup 的逻辑。我认为是合理的:因为既要把原来没试完的 backup 尝试遍,又应该追加新 change 的 backup 集合。

如果 changebackup 后想重新来(放弃没尝试过的老的 backup)应该在 auth 里主动 changehost ,而不是 error 。如果是 error ,按老的逻辑就直接连接建立失败了。

移到 mongo 的 auth 去做很难实现对:比如旧方案打的补丁就不完善。 auth 环节任何的 error 都会中断流程,没有地方可以驱动 socketchannel 切换 host 尝试下一组。因为 auth 本身很难保留状态到下一个 backup host 的连接尝试过程。旧补丁把状态留在 mongo.__pickserver 里原本就是个不太好的做法。

@sabearcher
Copy link

mongo.lua mongo_auth里

if rs_data.hosts then
   ....
   mongoc.__sock:changebackup(backup)

这一处我有疑问,既然socketchannel已经一开始就根据配置载入了全部的 可用节点,为什么要在连上其中一个节点时改变 __backup 呢?而且还可能出现该节点的hosts本身不完整的风险

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 12, 2020

因为客户端比服务器更不可信任,客户端的列表是更早配置的,和服务器集群间的协商结果相比,更加不可靠。

@sabearcher
Copy link

@cloudwu 那么可以用 addhosts , 比全部替换还是稳一点

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 13, 2020

不。可能就是需要删除一个坏掉的节点。客户端不应该为配置失误买单,应该信任服务器集群的决定。如果集群实现错误,那么是服务器的 bug 应该修正。

目前的做法是在一轮尝试中,在找到主节点前采用添加的语义。等到下一次,通常就采用了主节点定义的列表了。

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 13, 2020

@sabearcher 我想了解一下这个 pr 是否解决了你的问题,有没有新的问题。并下一个决定看要不要合并到主干。

@sabearcher
Copy link

sabearcher commented Jan 14, 2020

@cloudwu 我们现在正在尝试内部生产环境模拟阿里云正式服的异常,暂时还没有模拟出来,目前只能依据此前的log推断,可能阿里云的mongo有错误,使得某一次hosts返回的数据有误(我目前缺乏数据,只能推测是连上的节点1 正好是刚才出现故障重启的节点,会不会因此hosts有问题), 然后skynet表现为只尝试1~2节点,反复循环也没有遍历conf.rs里客户端配置的3~4节点。
正如你所说,客户端不应该为配置失误买单,但是如果配置在第一个节点就失误了(比如配错了服务器),那么问题其实更麻烦。而且,目前我们是不想为阿里云服务的错误买单,我们只是想要项目工程的稳定,因此这个解决方案恐怕还是有风险。

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 14, 2020

根据你之前的描述,“skynet表现为只尝试1~2节点” 并不是因为第 1 或第 2 节点返回了错误了 hosts 列表,而是第 2 节点可以建立连接却在 auth 阶段断开了服务。

之前的逻辑是,当 auth 出错就认为 socket channel 无法正确握手,会导致整个 socket channel 握手流程重置。充值之后就从头开始了,而没有迭代列表中的下一个。表现出来就是重复尝试 1,2 。

目前的逻辑,socket channel 在建立连接成功之前,视为一个连续的握手过程,在这个过程中,所有 changebackup 的调用都是向这个握手过程 添加 备选 host ,而不是替换。替换的是将来发起新的流程时的列表。

@sabearcher
Copy link

明白了,需要socketchannel重新执行 connect_once才会再用_add_backup 替换 addr_list备选。
目前看来应该是对的

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 14, 2020

我怀疑是:你的集群 2 号节点已经下线,但端口还开着,并可以连上去,但不能行使 mongo server 的职责。如果有机会遍历到第 3 节点,根据 mongo 集群的协商规则,可能就把 2 号节点踢出列表,或换成了正确的新的 ip 。

但是之前的 socket channel 不能处理这种情况,遍历到 2 号时,成功建立了 socket 连接但无法把 mongo 握手做完,导致了不断从头尝试。

你可以根据以上的描述来模拟出错环境。(即把 2 号节点的 mongo 服务停掉,但开一个服务在同样的端口上,这个服务只要有人连上来就主动断开)

@cloudwu
Copy link
Owner Author

cloudwu commented Jan 14, 2020

我觉得阿里云上的 mongo 集群之所以有这样的问题,可能是因为开放给你们的 ip 并不是 mongo 实际的服务,它只是一个代理。如果代理背后的真正服务挂了,表现出来就是可以建立连接(连上代理)却无法工作。

@sabearcher
Copy link

好的,我试试

@cloudwu cloudwu merged commit da43607 into master Jan 19, 2020
@cloudwu cloudwu deleted the socketchannel branch January 19, 2020 12:22
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.

3 participants