-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AT模式在oracle和postgresql中回滚逻辑实现不正确。 #4727
Comments
其次你得代码应该不是在1.5.1上测试的,具体是哪个版本? |
之前在读已提交下的时候就有类似问题,由于锁不住间隙,导致before和after不一致的情况,我们讨论过是否改写用户的update的条件,但是当时的方案是被否决的,我们不应该操作用户的sql,这会导致用户在排查问题上,或者打印sql的日志上觉得很奇怪,这个行为有点越权,而我们应该是让用户去调优自身的sql,比如你说的那个update id in的问题,如果是用户侧直接将条件改为id是否也能解决这个问题? |
@a364176773 @slievrly |
因为seata工作在底层,这是一个隐形的bug,只有出故障了才能发现。当然修改代码逻辑和sql语句,先查后修改,可以解决问题,但是这样AT模式就会让人很迷惑,什么时候可以用,什么时候不可以用? 对于这样sql语句应该怎么检查, before 和after的数量是一样的,而且有可能id也是一样的,但是更新的记录却不一样。
update t_test set age = 10 where id = round( random() * 10) ; |
至于我提出的改写sql语句方案,可以尽可能地保留原始的条件,便于用户通过sql语句排查问题。 原始SQL
update t_oot set name = 'lisi' where NAME = 'zhang';
修改后:
update t_oot set name = 'lisi' where id in (?, ?) and 'NAME = ''zhang''' <> 'transformed by seata' 这样修改后,仍然能够通过变换后的sql语句找到原始sql的语句。 |
修改用户侧的方案在mysql rc下锁不住间隙导致的问题我们内部在20年年底就已经讨论过了,方案就是改用户sql才能比较好的得到保障,但是由于这会给用户带来困扰,且可能出现不可预期的错误,我们否定了这种越权的方案,目前更倾向于,校验before count和update count,如果update>before抛异常来处理,至于你说的这种情况概率非常小,我们尽可能在文档和异常输出中指导用户尽量使用id作为update的条件,否则seata面临的是更多的背锅可能性,无论是答疑量,还是业务抛异常率先都会更觉得是seata侧引发的 |
你说这这种sql应该去阻断,而不是支持 |
我们一开始这块是为了防止修改主键,如果修改了主键那么前后镜像就会不同就通不过校验了 |
现在的阻断逻辑是错误的,afterImage使用的select …… where id in (?,?)来查询的,数量总是相等,根本不能阻断。 |
至于你说的修改sql导致的用户以为是seata的问题,其实一直存在,首先执行的是select …… for update,如果抛出异常一样会会让人觉得突兀。选择一个框架,首先应该了解其原理,因为即使用户自己写回滚,也是先select …… for update ,再update …… where id in (?,?),这个逻辑只是下沉到框架,交给seata自动实现而已。 |
看楼主的意思应该是oracle、PG的行锁不能锁住中间插进来的同条件的数据,导致目前前后镜像由于查询方式没有包含这部分数据,然后具体业务进行update操作的时候却这部分数据却参与了事务,出错回滚的时候这部分没有被正确回滚,如果是这样确实是一个很严重的BUG。 |
@slievrly @a364176773 @DoraemonAndRat |
@a364176773 @slievrly |
我只想知道,对于这个问题,seata是否有计划解决? 还是说按照你们所谓的设计政治哲学,就不管了。请给个回复,如果确实不打算解决的话,我会自己fork代码解决。 |
阻断判断的逻辑有问题,由于after image是根据before image的ID来进行查询的,所以只能判断出ID被修改或者数据被删除的情况,新增的数据是永远也判断不出来的。 |
@slievrly @a364176773 @DoraemonAndRat |
我什么时候说阻断逻辑是对的?我们阻断是阻断修改了主键导致不一样的情况?我说这种情况应该想办法阻断,而不是修改应用sql,这个问题在20年年底跟读已提交下锁不住间隙发生的行为是基本相近的,我们倾向是阻止,而不是改变用户的sql,至于你说的会前后select,这根本不算改变应用原有sql,这就像是环绕切面一样,并没有改变业务执行行为,如果你觉得有此类问题at不满足需求,你可以改用xa模式,至于提issue,不要带着这么大火气,这是开源组件,真如你所说,你可以自己fork改,但是不要没看清我的回复就自己yy代入 |
至于你确信自己用的是1.5,我只能告诉你要么你贴的代码块错了,要么是自己引错了 |
还有我提个问题,oracle跟pg都是默认rc隔离级别,如果你改成rr(pg),这个问题还存在吗?是否经过测试? |
1,不管你是用阻断还是其他什么手段,只要能保证正确性就行,问题是现在不能保证正确性。 |
我已经修改完代码了。rc级别有更好的性能,因此不需要再额外测试rr隔离级别能不能用。如果at模式只能再rr模式下使用,请再文档中标注一下,我在文档上是没有看到这个说明,也许我是文档看的不够仔细。 |
我在issue中代码复制错了,复制的代码是1.4.1的(以前用这个版本后来升级了),这是我的粗心造成的,给你造成困扰。 |
我把我的实现逻辑写一下。虽然不符合你们的设计哲学。 1,查询
变换前:
select id, name from t_test where age = 10 for update。
变换后:
锁阶段:
select id from t_test where age=10 and 'select id, name from t_test where age = 10 for update' <>'transformed by seata' for update;
执行阶段
select id, name from t_test where id in (?,?) and 'select id, name from t_test where age = 10 for update' <>'transformed by seata'
2,修改:
变换前:
update t_test set level = 1 where age < 10
变换后:
锁阶段:
select level from t_test where age < 10 and 'update t_test set level = 1 where age < 10' <>'transformed by seata' for update
执行阶段
update t_test set level = 1 where id in (?,?) and 'update t_test set level = 1 where age < 10' <>'transformed by seata'
2,删除:
变换前:
delete from t_test where age = 10
变换后:
锁阶段:
select * from t_test where age = 10 and 'delete from t_test where age = 10' <>'transformed by seata' for update
执行阶段
delete from t_test where id in (?,?) and 'delete from t_test where age = 10' <>'transformed by seata'
|
1.我没说20年已经有了,1.5上就没有这个问题了,我说的是20年的时候已经讨论了这个问题了,能仔细点看下对话吗?问题的结论是我们应该阻断这种危险行为的发生,而不是改用户的sql去支持 |
我已经用自己的方法解决了这个问题,同时删除了多余的afterimage查询,因为已经不需要了。阻断的方案我总觉得不踏实,总觉得会有漏网之鱼。我的观点刚好相反,不是黑名单,而是白名单机制,对于不需要修改sql的,不去修改它,否则就必须修改;不能识别的sql,必须修改,这样逻辑上才是完备的,心理才不抖。随着版本迭代,能识别的会越来越多; 对于数据库函数的阻断,我建议可以参考redis的lua脚本实现。 |
我说的难道不就是白名单?pr内容你倒是看一下呀,afterimage是需要的,主要是做后镜像检验,如果你能保证所有写入口都由globaltransactional注解进入,且由at模式运行,那么问题不大,因为不会有脏写,seata的undolog其实是本地事务的分布式事务版本,把本地的undo redo拖到全局事务层面,不会随着一阶段提交就无法被使用了,由seata托管undo redo |
我发现你老是不喜欢把回复的内容看完,就发一些回复怼人,老是误解我说的话,我觉得我已经表达的够清楚了,而你总是感觉看一半,就急冲冲的想回怼,我们做的应该是对问题本质的讨论,我提出a你提出b,再从合理性,规范性,框架边界,用户需求,接受层度,多方位考虑,所以我说的都是倾向,这个倾向随着未来发展可能会变,但目前倾向于不动用户的sql,做到识别内容和条件,进行提前的阻断,未来可支持时再放开限制 |
globaltransactional 并不能解决此issue的问题,只能修改数据库的隔离级别为rr才能正确。rr会导致性能下降,理论上生成undo,一次beforeImage就已经足够。 |
你这个白名单和我说的白名单是两个概念。我说的白名单是从语法语义中得出一定是能够OK的sql语句,假设版本1只允许这样的:update …… where id = 3; 版本二进化一下,update …… where id = 3 and name = 'xx'; 版本3再进化一下:update …… where id = 3 or id = 4。不是靠简单的危险词排除,而是从语义语法中识别,一定是这种结构的才能通过。 |
.......不知道你再争什么,我说的是绕过globaltransational注解,直接对数据库修改造成的脏数据,或者存在其他系统不使用seata共用库表的,你直接去掉了redo,只留了undo,没有校验阶段,出问题了你也不知道 |
也不是不能这么做,你上面说应该增加白名单,你也没说是这种类型,总之是需要一个白名单来先行处理允许通过的sql这是没问题的,先看看我们目前方案能不能hold住,至于你这个issue后面会通过update后的后镜像校验方式来处理,还有提醒一句这是社区问题沟通,意见相左是正常情况,不要太争强好胜 |
Ⅰ. Issue Description
AT模式在oracle和postgresql中的生成回滚日志实现逻辑是错误的。
Seata的实现逻辑如下:
由于mysql的锁是通过在索引上加锁实现的,因此在mysql中是正确的。
oracle和postgresql锁是加在行记录上。因此导致实际update数,和select的结果不一致。导致在高并发下不能正确回滚。
正确的实现应该是:
在Oracle和postgreSQL中应在锁定主键id,不应该再次使用条件查询,用如下sql语句。
即使在mysql中如果where条件中使用了rand() ,now()之类的可变函数也是有可能导致错误的。因为select 和update的结果不一致。
Ⅱ. Describe what happened
If there is an exception, please attach the exception trace:
Ⅲ. Describe what you expected to happen
在oracle和postgresql中能够正确回滚。
Ⅳ. How to reproduce it (as minimally and precisely as possible)
如:使用测试SQL: update t_test set age = 10 where name = 'zhang'
Ⅴ. Anything else we need to know?
Ⅵ. Environment:
The text was updated successfully, but these errors were encountered: