Fix-RW-Conflict

[triump2020]

User description

What type of PR is this?

• API-change
• [x ] BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #16278 #16220 #11815

https://github.com/matrixorigin/MO-Cloud/issues/3235

What this PR does / why we need it:

1. fix r-w conflict : CN help TN to transfer deletes hosted in memory when txn on CN commits.

---

PR Type

Bug fix

---

Description

• Enhanced transferDeletesLocked function to handle commit logic, including updating and resetting snapshot timestamps.
• Added timing control to ensure snapshot updates occur only after a specified duration.
• Modified Transaction struct to include a start field for timing purposes.
• Updated handleRCSnapshot function to call transferDeletesLocked with the commit parameter.

---

Changes walkthrough 📝

Relevant files

Bug fix

txn.go Enhance transferDeletesLocked to handle commit logic and snapshot updates pkg/vm/engine/disttae/txn.go Added commit parameter to transferDeletesLocked function. Introduced logic to update snapshot timestamp and reset snapshot if commit is true. Modified GetChangedObjsBetween call to use either the latest timestamp or snapshot timestamp based on commit. Added timing logic to control the snapshot update process. +27/-3    types.go Update Transaction struct and handleRCSnapshot for commit handling pkg/vm/engine/disttae/types.go Added start field to Transaction struct. Modified handleRCSnapshot to call transferDeletesLocked with commit parameter. +2/-4

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes to transaction handling and snapshot management which are critical and complex areas. The changes are moderate in size but require a deep understanding of the existing system to ensure correctness and no regression in behavior.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The condition if time.Since(txn.start) < time.Second*5 might cause the function to exit early without updating the snapshot or resetting it when commit is true. This could lead to inconsistent states if not handled correctly.
	Performance Concern: The update and reset of snapshots are now conditional based on the commit flag and the time elapsed, which could introduce performance variability depending on the transaction duration.
🔒 Security concerns	No

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Rename the inner ctx variable to avoid shadowing the outer ctx variable The ctx variable is redefined inside the forEachTableHasDeletesLocked function. Consider renaming the inner ctx to avoid shadowing the outer ctx and potential confusion. pkg/vm/engine/disttae/txn.go [1149] -ctx := tbl.proc.Load().Ctx +tableCtx := tbl.proc.Load().Ctx Suggestion importance[1-10]: 7 Why: Redefining ctx inside the function can indeed lead to confusion and bugs due to variable shadowing. Renaming it would improve code clarity.	7
Enhancement	Extract the duration condition into a well-named constant for better readability The time.Since(txn.start) < time.Second*5 condition could be extracted into a well-named variable to improve readability and maintainability. pkg/vm/engine/disttae/txn.go [1135] -if time.Since(txn.start) < time.Second*5 { +const minDuration = time.Second * 5 +if time.Since(txn.start) < minDuration { Suggestion importance[1-10]: 6 Why: Extracting the condition into a constant would enhance readability and maintainability, making the code easier to understand and modify.	6