Code Review在TDSQL-C 的应用实践

背景介绍和难点

1.1 为什么重视Code Review?

结合下面这个例子,我们来谈谈为什么要重视code review。假设你作为新人刚入职,领导分配了一个需求,于是接下来做了下面这些事:

为了完成任务疯狂敲了三天代码,
你将一个包含大约 800行新代码的commit提交MR,
收到两条关于代码风格的意见,以及一个对某块代码不是很理解的疑问,
修复了代码风格问题并回答了reviewer的问题,接着reviewer通过了你写的代码,
把代码分支合并到 Master,自动化测试完成,没有异常发生,
此后 几个月,你一直战战兢兢,不知道代码何时会crash,以及以什么方式crash…….
听完这个例子我们是不是有共鸣呢?也许这就是发生在我们身边的真实例子。

我们将code review的作用归纳为以下几个方面:

给编码者带来良性的社交压力。你正写一个比较紧急的需求,团队对代码的单元测试有一个要求:凡是新增的代码,必须有完整的单元测试以及需要达到一定覆盖率。此时你是否会有这样的想法,为了应付测试工具的覆盖率要求,先写一点不那么有用但是能带来覆盖率的测试。但是,一旦想到你的代码将会有你的同事参与review,有没有为刚才的这种想法产生一丝丝压力?这种压力是良性的,会阻止你去选择这些隐患很大的“投机”行为。
提高代码质量和可维护性。通过review发现缺陷,统一代码规范,功能模块化等保障代码质量和可维护性。
查缺补漏,发现一些潜在的问题。比如性能是否存在瓶颈,平台兼容性,错误异常处理。
知识分享,review他人代码其实也是一个学习的过程,自己可以从中学习别人的优点,反思自己平常开发过程中的不足。
对需求的double-check,与开发前的方案评审形成闭环。
让别人易看懂,让代码可以传承。

大多数缺陷是在编码过程中产生,在测试中被发现和修复,随着功能的测试上线,修复缺陷的成本也成指数级增长。因此code review作为测试后上线前的阶段,发现缺陷和解决缺陷也变得尤为关键,有效避免缺陷留到线上造成巨大损失。

1.2 Code Review存在哪些通用性难点?

在了解了code review及其重要性后,接下来我们分析下code review的难点:

review代码本身并不困难,但是需要耗费不少时间和精力去了解项目背景,需求背景,方案设计,甚至还需要了解关联模块的相关信息。除了读懂代码,还需要找到代码中测试没有发现的潜在的问题,这对reviewer的专业素养有较高要求。
review一般要求尽快完成,避免持续太长时间因为双方记忆的遗忘造成review效率降低。假如此时你很忙,code review的工作在一定程度上会造成很大的负担。
虽然我们提倡尽量不要出现大的代码量提交MR,但是也不能牺牲功能的完整性。假如你一下子收到了几千行需要被review的MR。是不是感觉很崩溃?

了解完code review及其难点后,接下来简单介绍下TDSQL-C以及code review在TDSQL-C存在哪些难点。

1.3 TDSQL-C是什么?

随着互联网的发展,各种业务数据快速膨胀,用户对数据库计算和存储能力的需求日益增长。在应对业务需求持续增长时,传统数据库的迭代和优化已经变得举步维艰,而分布式架构的优势则愈发明显。借助计算存储分离的架构,新硬件优势,物理复制特点,分布式系统优势,云原生数据库 TDSQL-C对比传统MySQL具有高性能,低成本,大存储,主从复制延迟低,秒级扩缩容,极速回档,serverless化等优势。

前面讲了TDSQL-C相对传统数据库的优势,那么接下来讲讲code review在TDSQL-C存在哪些难点?从下面的对比图可以看出,传统MySQL的数据,逻辑日志,物理日志,元数据都是存在本地盘,主从管理各自的数据,通过逻辑日志进行主从同步。TDSQL-C分为计算层和存储层,本地不再存储任何数据,共享存储层数据,主从通过物理日志进行同步,存储层通过接受主库发送的物理日志进行回放生成数据及元数据,不再需要逻辑日志。架构的巨大改变带来了以下问题:

TDSQL-C基于开源MySQL(系统复杂,项目总代码数百万级别),重构了日志系统,IO模块,事务模块,启动流程等多个模块,代码改动量巨大
TDSQL-C项目包括计算层,存储层,分布式文件系统,管控,运维等,参与人数众多,代码改动牵一发动全身
复杂的系统对研发,测试,code review都提出了极高要求

TDSQL-C Code Review流程

作为一个比较大的项目团队,经历多年的发展和优化,我们形成了目前的研发流程:

基于master分支创建属于自己的分支;
在自己的分支上进行开发;(开发之前要记录详细的设计方案,邮件的方式发送所有相关人及相关领导)
进行单元测试,性能测试,长稳测试,使用valgrind工具进行缺陷检查
上述测试通过后提交MR,自动触发单测,静态缺陷检测,生成覆盖率报告
全量覆盖率和增量覆盖率均需要达到90%以上才合格。
通知reviewer进行code review,并提供issue单,设计文档和测试数据,author和reviewer进行接下来的code review
code review通过后合入master分支
出包,正式提测

在这个过程中,对author和reviewer有不同的要求:

对author的要求

每次提交code review的代码量要少
以文档或其它方式向reviewer介绍修改了哪些文件,增加了什么样的功能

对reviewer的要求

把程序跑起来,调试一下是否符合预期,
尽可能及时的进行 code review,
在你读代码之前,先想想自己会怎么做,

Code Review主要参照以下几个方面进行:

代码书写规范,是否遵照代码规范进行书写,
代码实现与需求文档是否一致:核对需求单,
算法优化,思考最佳实现方法:if else的八二法则等,
细节把控:内存释放问题,错误处理,
注释是否合理,
单元测试是否完善,
代码提交commit规范

写好commit log很重要,它可以帮助我们快速了解这段代码的很多信息。为了从commit log中获取足够多的信息,我们对commit log有严格要求,每一个commit需要包括完整的信息:

类别:bug修复还是功能开发,
Issue:本地提交对应的issue,
主题:本次commit的概述,
Problem:要解决什么问题,
Solution:解决方法是什么,
MR:MR编号是多少,
Reviewer:记录reviewer同学

TDSQL-C CodeReview规范优化

前面介绍完了TDSQL-C的code review流程,接下来讲一下我们TDSQL-C在多年的code review实践中做的一些改进。

3.1 Code Review 的呈现形式

过去我们在评论里直接写上自己的意见,有时候给author没有传递准确的信息。

后来对Review的评论进行分级,不同级别打上不同的Tag:

[blocker]:代码行的问题必须要修改
[optional]:代码行的问题可改可不改
[question]:对代码行不理解,有问题需要问,author需要针对问题进行回复澄清

3.2 定期复盘

过去开发和线上稳定性维护的同学各司其职,缺乏有效反馈机制帮助开发人员了解线上会出现的一些通用性问题。

目前团队每周进行一次线上稳定性分析会,主要针对目前线上遇到的问题,讨论解决办法及后期如何避免,经验丰富的reviewer可以借助这些经验帮助author找到一些设计上,甚至是用户使用上可能触发的异常情况。

3.3 Code Review是一种开发文化而不是制度

Code review 的执行,很大程度上依赖于reviewer的认真审查,以及author的积极配合。过去往往流于形式,审查不够严格。

后来Code Review变成团队的一种文化,开发人员从心底接受并认真执行:

让开发人员认识到Code Review这件事为自己、为团队带来的好处,
团队负责人及资深工程师带头做好表率作用,
把code review作为开发任务的一部分,给author和reviewer都留出时间,
进行模块owner划分,每个子模块由资深工程师把关,提升效率,
代码合入master时需要注明reviewer,author和reviewer对代码承担同等责任,
团队定期组织topic share,加强技术分享

上一篇:Algorithm Review 4 跳表


下一篇:Egret 小游戏分包