nebula icon indicating copy to clipboard operation
nebula copied to clipboard

[BUG]Added support for data balancing when SSL is enabled. snapshot manager add ssl config

Open cfwl100 opened this issue 1 year ago • 3 comments
trafficstars

What type of PR is this?

  • [x] bug

What problem(s) does this PR solve?

Issue(s)

https://github.com/vesoft-inc/nebula/issues/5924:

Description:

数据均衡时,出现storaged节点间发送 Raft 副本的快照报错,且反复重试,任务一直长时间运行中,具备报错如下: 客户端报错

image

服务端报错

image 根据报错日志,追踪关键代码调用路径,SnapshotManager 管理和发送 Raft 副本的快照,承载创建Thrift客户端和发送数据能力,代码如下: image

ThriftClientManager中构造方法有两个,带参数和不带参数的,带参数的构造方法会传递入参enableSSL_,默认值是false Client方法中会根据enableSSL配置项走分支,当enableSSL为true时,创建AsyncSSLSocket(开启SSL),当为false时创建 AsyncSocket(未开启SSL)。

当客户端使用AsyncSocket,服务端使用AsyncSSLSocket进行解析时,会出现消息格式错误,因为客户端发送的消息未加密,最终服务端报错。 综上,SnapshotManager中enableSSL默认值为false,但是创建实例时使用了无参构造方法,导致这种致命性问题。

How do you solve it?

根据系统配置文件中的SSL配置项,将此配置设置到SnapshotManager带参的构造方法中,保证系统整体客户端和服务端SSL配置一致。

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

  • [ ] Unit test(positive and negative cases)
  • [x] Function test
  • [x] Performance test
  • [ ] N/A

Affects:

  • [x] Documentation affected (Please add the label if documentation needs to be modified.)
  • [ ] Incompatibility (If it breaks the compatibility, please describe it and add the label.)
  • [x] If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • [ ] Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe: Added support for data balancing when SSL is enabled.

cfwl100 avatar Aug 09 '24 12:08 cfwl100

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 09 '24 12:08 CLAassistant

@critical27 @dutor @SuperYoko Please help me with this issue,thanks

cfwl100 avatar Aug 16 '24 06:08 cfwl100

Sorry for late response. The cpplint in CI failed, please reformat the code~

critical27 avatar Sep 03 '24 06:09 critical27

Sorry for late response. The cpplint in CI failed, please reformat the code~

Has been fixed

cfwl100 avatar Sep 03 '24 07:09 cfwl100

@critical27 it's in a shape ready for merging 🤩

wey-gu avatar Sep 07 '24 22:09 wey-gu