五个例子品尝坏代码的味道

4,272 阅读6分钟

简介

最近项目性能方面挺窝心的,各种生产问题和压测不通过,仔细查看了各种代码,发现真不能推敲,救火也救不过来,希望通过这些反面教材,帮助你在日常开发中识别并避免坏代码,愿你我产出的代码都是保质保量,阿门。

文中列举的代码都以伪代码形式出现,毕竟工作代码不能外泄,大家理解意思即可,毕竟是什么不好的实践,也没必要深究

For循环查询MySQL

以下是查询用户信息的代码片段,可以看到设置权限列表时,通过用户ID查询角色列表,然后再遍历角色ID列表,查询每个角色对应的菜单权限,并合并去重后设置到用户信息UserInfo中。

public UserInfo findUserInfo(SysUser sysUser) {
       UserInfo userInfo = new UserInfo();
       userInfo.setSysUser(sysUser);
       // 设置角色列表 (ID)
       List<Long> roleIds = sysRoleService.findRolesByUserId(sysUser.getUserId()).stream().map(SysRole::getRoleId)
             .collect(Collectors.toList());
       userInfo.setRoles(ArrayUtil.toArray(roleIds, Long.class));

       // 设置权限列表(menu.permission)
       Set<String> permissions = new HashSet<>();
       roleIds.forEach(roleId -> {
          List<String> permissionList = sysMenuService.findMenuByRoleId(roleId).stream()
                .collect(Collectors.toList());
          permissions.addAll(permissionList);
       });
       userInfo.setPermissions(ArrayUtil.toArray(permissions, String.class));
       return userInfo;
    }

比在for...循环中依次查询,更好的做法是改为用in...查询,那么就仅仅需要对MySQL查询1次而已,更进一步还可以将角色对应的权限列表放入到Redis中,甚至都不用查询MySQL

接口实现极其重量

以下代码在一个方法体内实现了大量的重量操作,比如多次多表查询MySQL, 循环查询Redis,查询链路过长...而且单个方法实现过于复杂,只要数据量大到一定级别,该方法耗时将极高,如果要进行压测,一定是个很烂的接口。

public void complexQueryMethod(List<String> idList) { 
        // 1. 执行1次MySQL查询 
        ResultSet result1 = executeMySQLQuery("SELECT * FROM table1 WHERE condition1"); 
        
        // 2. 执行1次MySQL查询 ResultSet result2 = executeMySQLQuery("SELECT * FROM table2 WHERE condition2"); 
        
        // 3. 执行3~4次 join 3~4张表的MySQL查询 
        ResultSet result3 = executeMySQLQuery("SELECT * FROM table1 t1 JOIN table2 t2 ON t1.id = t2.id"); 
        ResultSet result4 = executeMySQLQuery("SELECT * FROM table3 t3 JOIN table4 t4 ON t3.id = t4.id"); 
        
        // 4. 执行1次MySQL查询,并for循环查询Redis,从Redis中拿数据 
        ResultSet result5 = executeMySQLQuery("SELECT id FROM table5 WHERE condition3"); 
        
        List<String> redisKeys = new ArrayList<>(); 
        
        while (result5.next()) { 
            String id = result5.getString("id"); 
            redisKeys.add(id); 
        } 
        
        // 查询 Redis 
        for (String key : redisKeys) { 
            String redisValue = queryRedis(key); 
            // 处理 Redis 数据 
        } 
        
        // 5. 执行1次 join了5张表的MySQL查询 
        ResultSet result6 = executeMySQLQuery("SELECT * FROM table1 t1 JOIN table2 t2 ON t1.id = t2.id JOIN table3 t3 ON t2.id = t3.id JOIN table4 t4 ON t3.id = t4.id JOIN table5 t5 ON t4.id = t5.id"); 
        
        // 6. 远程调用微服务B进行查询(假设微服务B的queryInfo也复杂,甚至还请求微服务C进行查询)
        ResultSet result7 = this.remoteService.queryInfo(idList);
        
        // 处理查询结果 
        processResults(result1, result2, result3, result4, result5, result6, result7); 
} 

// 执行 MySQL 查询的伪方法 
private ResultSet executeMySQLQuery(String query) { 
        // 实际的数据库查询代码 
        return null; 
} 

// 从 Redis 查询的伪方法 
private String queryRedis(String key) { 
        // 实际的 Redis 查询代码 
        return null; 
} 

// 处理查询结果的伪方法 
private void processResults(ResultSet... results) { 
        // 实际的结果处理代码 
}

这样的接口优化思路其实有很多

  1. 分而治之,将如此大而全的方法拆分成小而精的一个个小方法。
  2. 减少IO消耗,能一次请求查询完毕的,就不要分多次请求。
  3. 善用缓存,将各种结果集进行缓存,可以是Redis+JVM这样的多级缓存,特别在读多写少的环境下很有用。
  4. 比起在每次调用方法时才进行实时查询,更快的方式是在调用方法之前就已经根据业务需求提前把结果给计算了出来并存放,这样用户每次请求只需要直接读取结果集即可。
  5. 引用新的中间件,比如Elasticsearch,Flink,Spark等,搭建ETL架构也许是个不错的选择。

MySQL全表扫描

假设查询一张告警记录表,因为where 字段没有加上索引的缘故,导致了全表扫描

SELECT id, alert_message, alert_time 
FROM alert_records 
WHERE alert_time >= '2024-01-01' AND alert_status = 'ACTIVE'

explain结果

idselect_typetabletypepossible_keyskeykey_lenrefrowsExtra
1SIMPLEalert_recordsALLNULLNULLNULLNULL19889000Using where

type: ALL:表示全表扫描,即没有使用索引来优化查询。

possible_keys:表示可能用来优化查询的索引(如果存在的话)。

key:实际使用的索引(如果有的话)

rows:MySQL 估计的需要扫描的行数。

为了优化查询,那我们其实可以在 alert_time , alert_status上创建索引。掌握好MySQL最左匹配原则覆盖索引联合索引聚簇索引非聚簇索引概念很重要也很实在。

线程池使用错误

在方法内创建线程池,相当于每次调用这个方法都会创建一个新的线程池,只要大量请求该方法,即便该线程池任务队列有大小限制,也会引起因为创建太多线程而导致CPU飙升的危险

public List<RankingVo> queryRanking(RankingReq req) {
    // 创建一个线程池
    ExecutorService executorService = ThreadUtil.newExecutor(3);
    // 业务代码
    ........
}

对此优化只需要把ExecutorService声明为全局唯一的类变量或单例即可

产生MySQL长事务

以下是一个复杂且耗时的定时任务执行代码,注意在方法声明上添加了@Transactional 注解。 这样做的后果是

在逻辑复杂的定时任务中,一开始就begin开启了事务,然后定时任务里面有各种for循环,查询MySQL,HTTP请求...直到定时任务最后一行代码结束,才commit提交事务。造成漫长事务的等待提交,会导致很多别的针对所涉及的表的update、delete 操作出现 lock await超时的问题,出现大量的读写互斥

如果你执行以下SQL查询当前所有事务信息

select * from information_schema.innodb_trx;

发现有某些事务的开始时间trx_started距离当前时间比较久了,却还没结束,就该注意是否是一个大事务产生了

@Transactional
@XxlJob("RecordsJob.recordsJob")
public void complexProcessingMethod() {
    
    // 执行 MySQL 查询 
    ResultSet resultSet1 = executeMySQLQuery("SELECT id, name FROM table1 WHERE condition = " + i);
    
    for(int i = 0; i < resultSet1.size(); i++) {
        // 执行 MySQL 查询 
        ResultSet resultSet2 = executeMySQLQuery("SELECT id, key FROM table0 WHERE condition = " + i);
        
        // 处理结果 
        while (resultSet2.next()) {
            String id = resultSet2.getString("id");
            String key = resultSet2.getString("key");
            
            String httpResponse1 = performHttpRequest("https://api.example.com/resource?id=" + id + "&key=" + key );
            
            ResultSet resultSet3 = executeMySQLQuery("SELECT additional_data FROM table2 WHERE id = '" + id );
            
            while (resultSet3.next()) { 
                String additionalData = resultSet3.getString("additional_data");
                
                if (StringUtils.isNotBlank(additionalData)) {
                    String httpResponse2 = performHttpRequest("https://api.example.com/another-resource?data=" + additionalData + "&extra=" + id);
                    
                    // 处理 HTTP 响应 
                    processHttpResponse(httpResponse2);
                }
            }
            
        }
    }
    
    // 5. 执行独立的 MySQL 查询 
    ResultSet resultSet3 = executeMySQLQuery("SELECT * FROM table3 WHERE some_condition"); 
    while (resultSet3.next()) { 
        String info = resultSet3.getString("info"); 
        // 6. 执行与查询结果相关的 HTTP 请求 
        String httpResponse3 = performHttpRequest("https://api.example.com/final-resource?info=" + info); 
        // 处理 HTTP 响应 
        processHttpResponse(httpResponse3); 
        
    }
    
}

// 执行 MySQL 查询的伪方法 
private ResultSet executeMySQLQuery(String query) { 
        // 实际的数据库查询代码 
        return null; 
} 

// 执行 HTTP 请求的伪方法 
private String performHttpRequest(String url) { 
        // 实际的 HTTP 请求代码 
        return null; 
} 

// 处理 HTTP 响应的伪方法 
private void processHttpResponse(String response) { 
        // 实际的响应处理代码 
}

优化方法其实也很简单,只在真正需要更新MySQL的时候,才开启事务,尽可能缩短事务的生命周期,尽早提交。