Skip to content
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

fix issue #25, 在mvcc场景下,多事务执行删除会出错 #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ahbey611
Copy link

@ahbey611 ahbey611 commented Jun 1, 2024

修复了问题#25, 即lab5文档中的sql测试完成后,能删除id=0以外的数据了(如图1),删除id=0或delete from t_redo(删除全部数据,如图2)依然会触发写写冲突(因为transaction4的事务没有提交导致的)

image
53a24b2087b2e4745b5a300192aff96

@ycycse ycycse requested a review from RkGrit June 2, 2024 13:08
@hotwords123
Copy link
Contributor

感觉这样实现的主要问题是,需要对 RecordFileScanner 类做侵入式的修改,同时增加了模块之间的耦合。在之前的模块化设计中,RecordFileScanner 只负责记录扫描的逻辑,对查询过程的实现无感,具体的谓词过滤逻辑应该交给物理算子实现。此外,这个方法的正确性依赖于 predicates 下推优化的实现。

我想的解决方法是,在原先的代码框架下,不在 MvccTrx::visit_record 中判断并发冲突(因为此时只是判断可见性,并不一定真正删除或更新),而是直到实际调用 delete_record 时再执行判断,如果删除操作会导致并发冲突(删除了其它事务已经删除的数据)就返回错误。这样修改也更简单,把判断逻辑移动一下位置即可。

PR 中的代码可能有一些潜在的问题:

  • RecordFileScanner::fetch_next_record_in_page 只判断了 ComparisonExpr,但有可能存在其他类型的过滤条件,可以直接参考物理算子的实现,用抽象类 Expressionget_value 方法获取真值;
  • TableScanPhysicalOperator::open 调用 get_record_scanner 时把 predicates 移动走了,这样 close 后再次调用 open 会出问题(JOIN 场景下可能有这种需求),最好把所有 Expression 复制一份。

@RkGrit
Copy link
Contributor

RkGrit commented Jun 12, 2024

魏来同学的解决方案是将并发冲突的判断逻辑从 MvccTrx::visit_record 中移除,并推迟到实际调用 delete_record 时再进行判断。这种方式确实更符合逻辑,删除操作只有在执行时才需要判断并发冲突。这样不仅减少了侵入式修改,同时也更符合事务的实际执行流程。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants