那些年那些神码

首先说明一下什么是神码?神码就是神奇代码的意思(也是糟糕的意思),在这里是为了表达引以为戒!

往事不堪回首!想当年(2017年)公司技术团队新组建,系统新搭建。为了赶工期,一切以快速为目标,快速试错,快速交付上线。项目管理规范被忽视和技术规范管控没有及时跟上,工程师们交付的代码质量非常的糟糕。产生了不少严重的生产故障,后果比较严重,教训惨痛!

当年虽是架构师岗位,但却像是救火队员。毫不夸张地说是,哪里有生产故障问题,哪里就得去救火!

原因有三:

  • 团队新组建,成员水平参差不齐,有部分人还可以,但是有部分人基础确实不够硬,甚至不懂面向对象的人也来写java代码。
  • 赶工期,项目管理没有做好,没有制定和执行统一的代码规范。
  • 关键时刻容不得有半点怠慢,顶着巨大的压力,快速灭火救火才王道,使用问题影响最大化降低。

因此,在那些年救火过程中,填了不少的坑;事后复盘做了一些总结记录,针对问题进行深入分析,找出根因,希望避免再次出现,从而得到一些宝贵经验总结。

今天就来聊聊那些年给我曾经留下深刻教训的代码片断。在此不做批判,仅做反思与学习总结,也想让各位看官得到一点启发。下面我们来详细看看有哪些神码,到底有多神的代码?

神码片断1:不正确的使用redis命令

 

那些年那些神码

上面代码片断非常简单,就是简单封装的jedis工具类。初看也没有啥问题,但就因为这样代码导致生产出现奇怪的问题。生产环境部署多个业务系统,使用了同一个redis集群。某些业务系统的redis值被频繁清除,莫名其妙的丟失数据,排查很久之后才找出来。

最后通过redis后面服务监控,检查出来代码使用FlushALL命令,并通过全局搜索代码,找出来在某个业务系统退出登录的时候,调用了这个工具类。jedis工具类的init方法,init方法内部使用了flushAll命令,这个命令是会全库删除,非常坑的用法。当时是修改为flushdb(其实也有隐患的,如果多个应用或同个同个Redis db库,就会被刷掉)。

其实在用户退出的业务中,只需要清理相关对应的缓存就行了,即删除(del)对应的key值即可,完全没有必刷新动作。

 

神码片断2:不正确的使用@Transactional 事务注解

那些年那些神码

在这里代码片断中使用了Spring @Transactional 事务注解。

这一段代码里有三个操作:

第一是写入主业务表 save(customer);

第二是把相关数据写入附件表记录登录save(attachment);

第三远程调用一外部接口sendCustomerToWeChat(customer);

第一和第二个使用事务可以实现事务一致性,但第三用了一个异步线程,同时也跨服务的远程调用

    CompletableFuture.supplyAsync(() -> {
            sendCustomerToWeChat(customer);
            return "OK";
        });

这里事务是不能保证数据一致性的。

 

神码片断3:加了分布式锁也出现重复编码

那些年那些神码

看到函数加了 @Transactional 事务注解,同时函数内部加锁了redis分布式锁  RedisLocker.lock(lockName); 按理应该正常产生业务编码,结果其实不然,已经加了redis全局锁,但还是出现重复编码的情况

在高并发环境下可能会使用锁失效。正常做法是要么在事务外加锁,要么分解重写需要控制事务代码块。

锁失效的原因是:由于Spring的AOP,会在update/save方法之前开启事务,在这之后再加锁,当锁住的代码执行完成后,再提交事务,因此锁代码块执行是在事务之内执行的,可以推断在代码块执行完时事务还未提交;

其他线程进入锁代码块后,读取的库存数据不是最新的。

正确的做法要把最外层@Transactional 去掉。具体问题分析见《高并发环境下生成序列编码重复问题分析》。

 

神码片断4:跨服务调用数据列表导致内存溢出

 

那些年那些神码

那些年那些神码

公告模块查询逻辑非常简单,通过查出公告列表,然后根据当前人所在区域、组织、品牌品类、岗位进行数据集合的过滤。

如果说所有业务数据和人员架构、权限数据在同一个数据库,几个表join 一下结果很容易出来。

但现在的问题是,人员组织、权限是独立一个服务独立一个数据库;

业务数据公告又是独立的服务和数据库。也就是需要聚合两个服务list集合数据匹配过滤之后再进行结果的展示。

在大循环里面去查询部门、岗位、人员权限判断,然后通过远程RPC接口去调用人员接口数据。

每个人登录就将产生近1000次接口调用和本地数据业务查询组合,假定有1万人在使用,那意味着有1千万次远程调用,10万人访问,就有一亿次调用,面对巨大的网络IO,谁能扛得住,巨坑呀!

在测试环境测试的时候,访问人数少,没有测试出来,其实也是没有进行大规模的压测。

这一段代码上线后直接导致公告业务的服务应用内存溢出,服务死了好几次。坑死人不偿命!

阶段性优化修改:

那些年那些神码

循环调用之前,先把一些数据准备好,而不是进入循环里面去调用远程查询,减少跨机器的网络通讯时间和次数。优化改完之后,系统能正常运行,稳定下来了。

其实这种做法虽然阶段优化解决了问题,勉强过关,但仍然有很多改进优化的空间。

跨多个服务间调用:聚合——>条件过滤——>展示

多个List之间的聚合、遍历、拷贝,其实也消耗资源的,并发量高到一定程度,机器也承受不了。

优化方向转向使用ES,在发布公告即写入的时候就做一些平铺工作,把模板和权限逻辑做一些映射处理,查询的时候直接查询ES,然后做一些简单的标签符号替换,改造之后实现10万级别QPS,毫秒级响应。

ES改造后版本代码:

那些年那些神码

 

神码片断5:坑爹的类型判断

那些年那些神码

这种代码本质上代码规范问题,也是开发人员的基本素质问题。虽然不是什么致命问题,也产生正确的结果,但按照代码规范实在不应该这么写。

存在问题:

  • 字符串比较不要用"=="而是用equals;
  • 既然是判断是与否,就直接用boolean类型,增加代码可读性和健壮性;

稍微修改一下,不然真的无法看。

那些年那些神码

 

引申知识点:

基本数据类型它们之间用“==”比较时,比较的是它们的值。
引用数据类型它们用“==”比较时,比较的是它们堆内存地址。
Object equals()初始默认行为是比较对象的内存地址值,不过在String、Integer、Date等这些类中,equals都被重写以便用来比较对象的成员变量值是否相同,而不再是比较类的堆内存地址了。

看String equals JDK8源代码

那些年那些神码

对于Integer var = ? 在-128至127范围内的赋值,Integer对象是在IntegerCache.cache产生,会复用已有对象,对象引用地址是同一个,而这个区间之外的所有数据,都会在堆上直接产生新的对象。这是个大坑!!!

基本数据类型(如byte、short、char、int、long、float、double、boolean 等)的值比较,用 ”==” 进行比较。
引用数据类型( 如String、Short、Char、Integer、Long、Float、Double、Boolean、Date等)的值比较,用equals进行比较。
推荐使用java.util.Objects#equals(JDK7引入的工具类)

 

神码片断6:万恶的where空条件

那些年那些神码

这段代码很简单,也很好理解,可是发布到生产环境却造成严重的灾难,可称得是史上最严重的BUG,下面详细描述一下这过程发生细节。

一、问题产生过程描述:

  • 一个同手机号码用户(吴X兵)名下有多个账号,用户操作某些账号失效;
  • 然后用失效账号登录,能正常登录到系统,继续做修改手机号码的动作;
  • 修改手机号码时,由于程序查询逻辑不够严谨,主用户为空导致查询全表数据;
  • 全部用户数据更新为同一个手机号码,问题暴发!
  • 10:35左右发现UC系统比较卡,UC数据库有锁表时间过长告警,开发开始排查问题,11:20答疑收到终端用户(吴X兵)反馈收到很多(计审、价审)电话。
  • 通过查询数据库、日志和链路定位到问题,12:30左右发布修复补丁,并从备份数据恢复数据(前一天凌晨3点的数据),并刷数补齐上午产生的数差。
  • 1:30开始排查并修复各个业务产生的数据(服务单、设计软件任务列表、工厂订单、裂变活动、送货安装、MSCS订单);
  • 其中影响比较严重的是工厂订单,产生5万多条生产传单数据,其中2.5万多条流传到制造,准备到工厂车间排产。

二、详细排查问题记录

详细分析阿里云服务日志

2021-12-08 09:54:43.999

吴X兵一个正常B端用户登录我们平台,他在自己账号管理模块进行了解绑账号操作( 账号:CZJR022@xx09243)本来就是一个很普通很正常的业务操作,他也如期正常的操作完了。

那些年那些神码

解除绑定操作正常成功之后,系统内部会进行调用清缓存接口,系统日志显示如下:

那些年那些神码

解绑成功能之后,主账号MainUserId被清除掉了。

2021-12-08 10:38:08.528  

吴X兵,又进行操作修改本人的手机号操作

那些年那些神码

结果悲剧正常产生了,就是开头那段代码,where条件为空,相当于查询全表!从链路日志也可以抓到这个SQL

那些年那些神码

开始出现批量更新手机号这个主用户手机号。库里所有其他的账号全部被更新为这个吴X兵的手机号码,呜呼!!!!

那些年那些神码 

手机号码字段数据全量被更新为同一个,问题暴发之后,对此服务进行紧急灭火行动,对终端用户发布紧急停服通告,服务暂时挂起1小时进行数据修复。

由于这个服务没有做小时级别的数据增量备份,只能拿前一天数据凌晨3点的数据做数据库恢复,今天增量数据(900多条),只能通过解析系统日志,一条条从日志中找出来去匹配修复。

三、遗留的问题

  • 部分设计文件写入PDF和XML的已经固化,设计文件无法做更新,只能重新发起重新生成,真是悲惨!
  • 个别账号出现状态不一致情况,只能通过对比恢复前后数据进行更新刷数处理。

四、问题反思

  • 失效的账号仍然能登录,这是程序的一个大BUG。
  • 条件为空时查全表,需要大家吸取血的教训,举一反三,要求大家写程序时要严谨,加强自测,该加判断的不能少。

五、强化解决

  • 框架层面解决无效当前用户全局拦截校验,阻断具体的业务操作;
  • 加强代码,判空,非空,必填等核心逻辑代码对参数进行必要校验;
  • 切面AOP全局拦截查询、更新、删除等全表操作的SQL,对无参进行拦截阻断;
  • 重要数据质量安全监控,状态一致,数据一致性非常重要;
  • 数据备份策略优化改进,重要数据按时段多几个备份。

经过这一次惨痛教训,决定在框架层做点功能,把不符合规则的SQL拦截掉,即不带where条件参数SQL进拦截,具体代码如下:

@Intercepts(         {                 @Signature(type = Executor.class, method = "query", args = {MappedStatement.class, Object.class, RowBounds.class, ResultHandler.class}),         } ) @Component public class AllQueryInterceptor implements Interceptor {      /**      * 白名单:允许全表查询的表名      */     @Value("${white.table.name:}")     private String whiteTableName;      /**      * 允许不带where条件,只带limit,且limit的最大条数      */     @Value("${limit.size:10000}")     private Long limitSize;      /**      * 全局控制是否启动该校验的开关      */     @Value("${all.query.check:true}")     private Boolean allQueryCheck;      private static final Logger LOGGER = LoggerFactory.getLogger(AllQueryInterceptor.class);      private static final Pattern p = Pattern.compile("\s+");      @Override     public Object intercept(Invocation invocation) throws Throwable {         MappedStatement mappedStatement = (MappedStatement) invocation.getArgs()[0];         Object parameter = invocation.getArgs()[1];         BoundSql boundSql = mappedStatement.getBoundSql(parameter);          if(!sqlHavingWhere(boundSql) && !sqlHavingLimit(boundSql) && allQueryCheck){             LOGGER.debug(boundSql.getSql());             throw new BusinessException("检测到您有操作全表记录的风险,请联系系统管理员!");         }else{             return invocation.proceed();         }     }      private Statement getStatement(String sql){         Statement statement = null;         try {             statement = CCJSqlParserUtil.parse(sql);         } catch (JSQLParserException e) {             LOGGER.error("转换sql失败,原sql={}",sql);         }         return statement;     }      /**      * 判断是否有limit      * @param boundSql      * @return      */     private Boolean sqlHavingLimit(BoundSql boundSql){         try {             IPage page = getPage(boundSql);             if (null != page && page.getSize() >= 0L && page.getSize()<=limitSize){                 return true;             }else {                 String originalSql = boundSql.getSql();                 return originalSql.contains(CommonConstants.SqlKeywords.LIMIT);             }         } catch (Exception e) {             LOGGER.error("判断sql是否涉及全表操作异常,原因{}",e);         }         return true;     }      /**      * 判断sql是否涉及全表操作      * @param boundSql      * @return      */     private Boolean sqlHavingWhere(BoundSql boundSql){         try {             String originalSql = boundSql.getSql();             Statement stmt  = getStatement(originalSql);             if(null != stmt){                 // 允许全量操作的表在白名单放开                 if(whiteTableName(getTableNames(stmt))){                     return true;                 }                 // where没有条件或者只有一个删除标识条件,则认为是全表操作                 Set<String> where = getWhere(stmt);                 if(where == null){                     LOGGER.debug("疑似操作全表的sql={}",originalSql);                     return false;                 }else if(where!=null && where.size() == 1 && CommonConstants.SqlKeywords.DEL_FLAG.equals(where.iterator().next().toUpperCase())) {                     LOGGER.debug("疑似操作全表的sql={}",originalSql);                     return false;                 }             }         } catch (Exception e) {             LOGGER.error("判断sql是否涉及全表操作异常,原因{}",e);         }          return true;     }      /**      * 获取分页数据      * @param boundSql      * @return      */     private IPage getPage(BoundSql boundSql){         Object paramObj = boundSql.getParameterObject();         IPage<?> page = null;         if (paramObj instanceof IPage) {             page = (IPage)paramObj;         } else if (paramObj instanceof Map) {             Iterator var8 = ((Map)paramObj).values().iterator();              while(var8.hasNext()) {                 Object arg = var8.next();                 if (arg instanceof IPage) {                     page = (IPage)arg;                     break;                 }             }         }         return page;     }      /**      * 获取表名      * @param statement      * @return      */     private List<String> getTableNames(Statement statement){         List<String> tableNames = new ArrayList<>();         if(statement != null){             TablesNamesFinder tablesNamesFinder = new TablesNamesFinder();             tableNames = tablesNamesFinder.getTableList(statement);         }         return tableNames;     }      /**      * 判断表名是否在允许查全表的白名单内      * @param tableNames      * @return      */     private boolean whiteTableName(List<String> tableNames){         for(String tableName : tableNames){             // 有些表名带了``,把它去掉             if(tableName.startsWith("`") && tableName.endsWith("`")){                 tableName = tableName.substring(1,tableName.length()-1);             }             if(whiteTableName.contains(tableName)){                 return true;             }         }         return false;     }      private List<PlainSelect> getPlainSelect(Statement stmt){         List<PlainSelect> plainSelectList = new ArrayList<>();         Select select = (Select) stmt;         SelectBody selectBody = select.getSelectBody();         if(selectBody instanceof PlainSelect){             PlainSelect plainSelect = (PlainSelect) selectBody;             plainSelectList.add(plainSelect);         }else{             SetOperationList setOperationList = (SetOperationList)selectBody;             for(SelectBody setOperation : setOperationList.getSelects()){                 PlainSelect plainSelect = (PlainSelect) setOperation;                 plainSelectList.add(plainSelect);             }         }         return plainSelectList;     }      /**      * 获取where里面的参数      * @param      * @return      */     private Set<String> getWhere(Statement stmt){         Set<String> whereItemSet =new HashSet<>();         List<PlainSelect> plainSelectList = getPlainSelect(stmt);         for(PlainSelect plainSelect : plainSelectList){             getWhereItem(plainSelect.getWhere(),whereItemSet);         }         return whereItemSet;     }      /**      * 获取where节点参数      * @param rightExpression      * @param leftExpression      * @param tblNameSet      */     private void getWhereItem(Expression rightExpression,Expression leftExpression,Set<String> tblNameSet){         if(rightExpression != null){             if (rightExpression instanceof Column) {                 Column rightColumn = (Column) rightExpression;                 tblNameSet.add(rightColumn.getColumnName());             }if (rightExpression instanceof Function) {                 getFunction((Function) rightExpression,tblNameSet);             }else {                 getWhereItem(rightExpression,tblNameSet);             }         }         if(leftExpression != null){             if (leftExpression instanceof Column) {                 Column leftColumn = (Column) leftExpression;                 tblNameSet.add(leftColumn.getColumnName());             } if (leftExpression instanceof Function) {                 getFunction((Function) leftExpression,tblNameSet);             }else {                 getWhereItem(leftExpression,tblNameSet);             }         }     }      /**      * 获取where里面的字段      * @param      * @return      */     private void getWhereItem(Expression where, Set<String> tblNameSet){         if(where instanceof BinaryExpression) {             BinaryExpression binaryExpression = (BinaryExpression) where;             Expression rightExpression = binaryExpression.getRightExpression() instanceof Parenthesis?((Parenthesis) binaryExpression.getRightExpression()).getExpression(): binaryExpression.getRightExpression();             Expression leftExpression = binaryExpression.getLeftExpression() instanceof Parenthesis?((Parenthesis) binaryExpression.getLeftExpression()).getExpression(): binaryExpression.getLeftExpression();             getWhereItem(rightExpression,leftExpression,tblNameSet);         }else if(where instanceof Parenthesis){             getWhereItem(((Parenthesis) where).getExpression(),tblNameSet);         }else if(where instanceof InExpression){             InExpression inExpression = (InExpression) where;             Expression leftExpression = inExpression.getLeftExpression() instanceof Parenthesis?((Parenthesis) inExpression.getLeftExpression()).getExpression(): inExpression.getLeftExpression();             getWhereItem(null,leftExpression,tblNameSet);         }     }      /**      * 获取select里面function里面的字段      * @param function      * @param selectItemSet      * @return      */     private void getFunction(Function function, Set<String> selectItemSet){         if(function.getParameters()==null || function.getParameters().getExpressions()==null){             return;         }         List<Expression> list=function.getParameters().getExpressions();         list.forEach(data->{             if (data instanceof Function) {                 getFunction((Function)data,selectItemSet);             }else if (data instanceof Column) {                 Column column = (Column) data;                 selectItemSet.add(column.getColumnName());             }else{                 getWhereItem(data,selectItemSet);             }         });      }

 

神码片断6:地狱式18层 if-else-for嵌套

 

那些年那些神码

像上面这种18层地狱式代码,看完是不是很想吐血!这里篇幅限制问题仅展示其中一小段,这种神码早些年我们旧项目中巨量存在。

这也是前人留下来宝贵的手笔,这种代码完全毫无设计,写这代码的人不讲武德,当时写这些代码的作者因为一些原因离职了,我们当年系统上线后将近一年多的时间里不敢去修改这神代码!自从接手那一天起,受尽各种艰难折磨,心中的苦只有自知,难受!

业务要增加需求吧,我们说这需求加不了,暂时搞不定,等系统重构版本出来之后再来提新需求。业务不理解天天诟病,天天叫骂,之前都可以的,怎么现在就不行了。哈!哈!哈!

业务反映的BUG吧,我们硬得头皮,只能再火坑里面加点油,花大量时间去研读作者的写作意图,然后小心奕奕做点局部修改,大家每次改完BUG心里,测试、发布、上线心里那个忐忑呀!

后面终止下大决心,对项目进行重构,经过两次大版本重构之后,无数次的修正,终于把整个仓库封存起当作纪念品!

确切地说我们是通过领域驱动设计方法,彻底解放了这种神码,变废为宝!具体怎么做的可以参考另一篇《我是这么玩领域驱动设计的DDD

 

总结

1、上面仅列了一小部分典型的神码,还有很多没贴出来;主要是经过多次重构设计之后,神码慢慢消失在历史长河之中。还希望各位看官们多总结多分享,并从中得一点启示。

2、实际工作中神码无处不在,在神码世界的里,你永远有可能收获意想不到的惊奇;为了减少工作中烦恼,为了美好的生活,写代码时候多点思考和设计。

3、一个复杂的项目往往由团队多人分工合作完的,团队需要建一套严格的代码规范约束,老鸟们多做codeReview,并贯彻始终,否则团队协作交付成果将大打折扣。

4、作为码农自身需要不断地加强武德修养,交付良品,拒绝交付废品;最直接的目的就一条为了不让后人鄙视和诟病就够了。

 

发表评论

相关文章