同事的问题代码 点击关注公众号，“技术干货”及时达！ 前言??先祝各位老铁新年快乐啊?? 快过年了，趁着最近活不是很多，今天和大家一起对我们的项目进行code review一下，让各位老铁都体验一下当“技术总监” 的 ??feeling??。 问题代码优惠券领取写优惠券领取代码的大哥还是有实力的，工作经验至少也有8年以上了，毕竟都30好几的老大哥了?? ?这大哥还是有点东西的，自己实现了一个简单的 锁管理工具 使用了 ConcurrentHashMap、单例模式、AtomicBoolean、volatile+双检锁 实现的单例, 看注释还差点用上了弱引用 ?我们就先看看controller 层和 核心的service代码吧！?? 原代码逻辑「Controller层:」 看一下 Controller 层的伪代码,主要的逻辑： 首先接口有限流注解用户校验数据库领取核心业务，库存扣减 （核心逻辑）推送领取成功记录（告知第三方）@RedisRateLimiter(value=200,limit=1) @PostMapping(value="/claim") publicObjectclaim(@RequestBodyEquityClaimReqExclaimReqEx){ //1、用户信息验证 Resultresult=claimService.check(claimReqEx); if(!result.isSuccess()){ returnresult; } //2、领券--独立分段事务 ResultclaimCore=claimService.claimTran(claimReqEx); if(!claimCore.isSuccess()){ returnclaimCore; } //3、记录领取明细 returnclaimService.record(claimReqEx); } ?Controller 层居然没有加锁 ?「service核心代码：」 看一下service代码的处理核心逻辑，实际代码逻辑还是比较复杂，个人写了伪代码 随机获取一个优惠券?随机获取优惠券的逻辑是用的SQL的 limit 1，多少还是可以优化的吧！ ?给优惠券加锁，获取锁失败直接返回，用的 redis setnx特性锁库存，锁的逻辑还是自定义封装的锁工具??给我看麻了啊查询库存库存 并扣减 ，失败则回滚更新优惠券码的领取状态、和用户领取状态，失败则回滚@Transactional(propagation=Propagation.REQUIRED,isolation=Isolation.READ_COMMITTED,rollbackFor=RuntimeException.class) publicResultclaimCoreTransactional2(ActivityEquityClaimReqExclaimReq){ longstartTimeCore=System.currentTimeMillis(); //------第（1）步：随机获取一个未发放的券码 //原作者注释：进行商品信息获取:通过权益、站点、活动id,获取唯一商品 EquityGoodsequityGoods=equityGoodsDao.findByClaim(claimReq.getActivityId(),claimReq.getEquityId()); if(null==equityGoods){ returnResult.error("权益商品异常：无可领取权益商品！"); } //--------第（2）步：给券码加锁 //原作者注释：针对同样的用户暴力请求，不做阻塞等待，直接异常返回处理 Booleanacquired=redisTemplate.opsForValue().setIfAbsent(equityGoods.getRedeemCode(),"Lock",Duration.ofSeconds(60*5)); if(acquired){ try{ //-------第（3）步：锁库存 //原作者注释：获取自定义锁对象（这里根据业务清理考虑奖券领取情况） ClainReentrantLockclaimLock=claimLockManager.getLockForClaim(getLockKey(claimReq),60*24,TimeUnit.MINUTES); BooleanisCompleted=false; //原作者注释：1、减库存、商品获取、领取情况核心逻辑保证数据一致性 try{ claimLock.lock(); //--------第（4）步：查询库存库存并扣减，扣减失败回滚，成功进入下一步 Inventoryinventory=inventoryDao.findByconfirm(claimReq); booleanisok=inventoryDao.reduceInventory(inventory.getId(),isCompleted); if(!isok){ thrownewRuntimeException("更新配置权益核销状态失败！"); } //----------------------第（5）步：更新优惠券码的领取状态、和用户领取状态，成功则完成领取，失败则回滚 Booleanret=updateUserRecordAndRedeemCode(); if(!ret){ thrownewRuntimeException("对不起，系统繁忙，稍后再试试吧！"); } }catch(Exceptione){ e.printStackTrace(); throw }finally{ //释放锁资源 claimLock.unlock(); if(isCompleted){ //站点权益分配完成，标记当前锁 claimLock.markForCleanup(); } longendTimeCore=System.currentTimeMillis(); logger.info("================领取逻辑耗时测试.Timed:{}",endTimeCore-startTimeCore); } returnResult.ok(); }finally{ //释放锁 redisTemplate.delete(equityGoods.getRedeemCode()); } }else{ returnResult.error("对不起，系统繁忙，稍后再试试吧！"); } } ?居然用自己写了一个锁的管理工具??就是后来人不好维护啊?? ?存在的问题核心service层算是实现得比较复杂的了吧，先是锁券码，然后是锁库存，库存扣减也是利用 SQL set num = num-1 where id =? 去扣减（数据库行锁，所以扣减肯定是安全）。因为库存锁是写在事务里面的，可能引起事务还没提交，所已经释放了，所以还是有并发风险（重复领取）。 限流注解的实现大有问题，往期文章说过这个问题，详情见：同事代码问题（第三篇） Controller层的用户校验是没有加锁的，有没有可能，第一个用户并发请求发生重复领取呢，建议还是在Controller 层 做一个 userId 锁吧。 ?给userId 加锁 防住用户重复点击，重复领取等问题，并发请求。控制单人并发，提高系统整体的有效并发请求。 之前也有同事，在Controller 层直接对库存加锁的，这样同一时刻 只能有一个有效请求，用户体验不好，但是能减少数据库压力。给userId加锁，用户多的话，压力给到数据库了（秒杀场景的话，库存扣减大多现在redis中lua脚本执行 ）。 ?利用limit 1 返回一个可领取的优惠券，这样多个用户进来，很大概率 获取到的都是同一个优惠券，这样大多数的请求就成无效请求了，用户体验不好 ?（当然实际业务场景，没啥并发，这里也是为了技术而技术了）。 ?锁问题,锁的券码，券码的查询逻辑是limit 1。注释写的是防止单个用户暴力请求，多少有问题，防止用户重复请求可以 锁userId啊！//原作者注释：针对同样的用户暴力请求，不做阻塞等待，直接异常返回处理 12 Boolean acquired = redisTemplate.opsForValue().setIfAbsent(equityGoods.getRedeemCode(), "Lock", Duration.ofSeconds(60*5)); 自定义的锁管理工具，自己爽了?，别人难受了啊?? 一个方法里面 两个try，多少感觉不对头啊！一个就可以了吧 事务里面加锁，存在并发问题，事务还没提交，锁已经释放了，所以存在锁失效的可能 优化方案接口做了限流之后，我们的锁就只需要锁用户就行了，库存扣减 和 券码领取 的状态修改，交给数据库处理就行了，数据更新行的时候会进行锁，并发不大的话，数据库也能抗住，关键是代码简化了许多。 Controller 直接 对userId加锁，获取到不锁直接fastfail，当然错误的限流实现也得修复（详：同事代码问题（第三篇）） @RedisRateLimiter(value=200,limit=1) @PostMapping(value="/claim") publicObjectclaim(@RequestBodyEquityClaimReqExclaimReqEx){ //对领取业务的用户ID加锁 Booleanlock=redissonLockClient.tryLock(RedisKeys.COUPON_RECEIVE_LOCK+user.getId(),20); if(!lock){ return"服务繁忙....."; } try{ //1、用户信息验证库存校验 Resultresult=claimService.check(claimReqEx); if(!result.isSuccess()){ returnresult; } //2、领券--独立分段事务 ResultclaimCore=claimService.claimTran(claimReqEx); if(!claimCore.isSuccess()){ returnclaimCore; } //3、记录领取明细 returnclaimService.record(claimReqEx); }finally{ //解锁 redissonLockClient.unlock(RedisKeys.COUPON_RECEIVE_LOCK+user.getId()); } } service 层 以前的券码锁、库存锁直接 去掉，获取券码的逻辑 从redis 获取或者 是 线程安全的队列中获取. 更新劵码的状态时候，需要在where 条件中 添加status='未领取',如果更新失败 就回滚事务。 ?当然如果从从redis 或者 队列 中获取的话，我们就得去我们缓存数据了，比如券码已经被消费了、或者消费失败了 就要去删除缓存 或者是重新添加到缓存中。 ?@Transactional(propagation=Propagation.REQUIRED,isolation=Isolation.READ_COMMITTED,rollbackFor=RuntimeException.class) publicResultclaimCoreTransactional2(ActivityEquityClaimReqExclaimReq){ longstartTimeCore=System.currentTimeMillis(); //------第（1）步：随机获取一个未发放的券码从队列 ConcurrentLinkedQueue goodQueue 或者是 redis set 中获取一个随机券码， //从队列中或者 redis 中获取一个券码，不用用户同一时间进来能获取到不同的券码，提高请求效率，和并发量； EquityGoodsequityGoods=goodQueue.poll(); if(null==equityGoods){ returnResult.error("权益商品异常：无可领取权益商品！"); } //第（2）步：更新券码状态为领取 //`setstatus=2（领取）wherestatus=1(未领取)andid=xx` booleanupdateRet=updateGoods(id); if(!updateRet){ thrownewRuntimeException("更新配置权益核销状态失败！"); } //--------第（3）步：查询库存库存并扣减，扣减失败回滚，成功进入下一步 //`setnum=num-1whereid=xxandnum0` Inventoryinventory=inventoryDao.findByconfirm(claimReq); booleanisok=inventoryDao.reduceInventory(inventory.getId(),isCompleted); if(!isok){ thrownewRuntimeException("更新配置权益核销状态失败！"); } //----------------------第（4）步：更新优惠券码的领取状态、和用户领取状态，成功则完成领取，失败则回滚 Booleanret=updateUserRecordAndRedeemCode(); if(!ret){ thrownewRuntimeException("对不起，系统繁忙，稍后再试试吧！"); } returnResult.ok(); } 审批业务这个就简单了，主要就是两个操作 提交审批 和 审批 和 撤销三个操作，实际上也没啥大问题。先看看代码吧，看看可能存在什么问题?? 原代码逻辑「提交审批」 更新主表的审批状态为 待审批插入一条待审批的日志 到日志表「审批」 更新 日志表 的审批状态更新主表的审批状态@Transactional(rollbackFor=Exception.class) @Override publicvoidapprove(LbApproveLogUpdateReqreq){ IntegerupdateCount=logMapper.updateApproveStatusByIdAndApproveStatus(req); if(updateCount=0){ thrownewJeecgBootException(NewLaborConstant.MSG_ERROR_APPROVE_OPERATE_FAIL); } //修改主表记录。 baseMapper.updateApproveStatus(req); } 「撤回」 更新 主表更新日志表@Transactional(rollbackFor=Exception.class) @Override publicvoidapproveCancel(Longid){ IntegerupdateCount=baseMapper.updateApproveCancel(id); if(updateCount=0){ thrownewJeecgBootException(NewLaborConstant.MSG_ERROR_APPROVE_CANCEL_OPERATE_FAIL); } //删除审批记录表信息。 logMapper.removeByMainIdAndApproveStatus(logId); } 存在问题看起来确实没啥问题，都搬到台面上来了，怎么也得鸡蛋里面挑骨头?? 存在的问题就是，「审批」的时候 更新顺序是 日志表--主表 「撤回的时候」 是 主表----日志表 ??同时操作同一个业务数据，容易造成死锁呀，毕竟方法都加了事务的，行锁需要整个事务执行完成才能释放 优化方案对于这种需要在事务里面 更新多表的 代码，尽量更新的顺序一致吧?? 总结分享了两个案例，一个是优惠券领取，一个是审批、撤回业务。在优化券领取场景中，「锁对象」还是很有讲究的，以及「锁和事务」 的配合使用 ，使用不当还是存在并发的问题。后面的审批业务，分享了在事务中 因为更新顺序不一致 可能引发「死锁」的问题。 希望本篇文章能对你有所帮助，感谢各位老铁，一键三连呀?????? ?ps 往期文章： 同事的代码问题(java) 同事的代码问题（第二期） 同事的代码问题(第三期) ?点击关注公众号，“技术干货”及时达！ 阅读原文