pingcap / pingcap/tidb

Refactor txn management season2

Open
#41,098 0 comments 0 reactions 0 assignees View on GitHub
sig/execution sig/sql-infra sig/transaction type/enhancement
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Motivation

After issue #30535 on https://github.com/pingcap/tidb, the transaction interface has been abstracted into `TxnManager` and `TxnContextProvider`. As a result, the transaction utilities used by both `session` and `executors` have become more straightforward and tidy.

The current management of transaction status is disorganized and complex due to inconsistent access interfaces. For example, `TxnManager` provides access to transaction status information, activation of transactions, and refreshing `for_update_ts`. However, reading and modifying transaction status can also be done through `sctx.sessionVars.TxnCtx`, while some transaction statuses are included in `tikvTxn`. This complexity makes it difficult for users to obtain or modify a specific transaction status without errors. To improve code maintainability and facilitate future feature developments, we recommend refactoring the code related to transaction status.

## Plan

### Unified Transaction Status Information

At present, the `TransactionContext` structure stores information about transaction status. This structure is created and initialized during transaction initialization, and each transaction has its own independent structure.

TODO

### Unified Transaction Status Access Interface

At present, the reading and writing of transaction status primarily rely on `TransactionContext`, with some code accessing it through `TxnManager`. This multiple access approach makes modifying the code more challenging. For instance, when implementing the "Fair Lock Model" optimization, due to numerous read/write entry points, making changes to `for_update_ts` was particularly difficult.

- [ ] The usage of the `forUpdateTS` interface has been standardized by obtaining it uniformly from the `TxnManager` interface and removing direct invocation from `TxnCtx`. Additionally, an abstract interface was created to solely read the current status information of `forUpdateTS`, without triggering transaction activation or refresh operations.

TODO

Contributor guide

Open the contributing guide

Research direction

Start by reading the TransactionContext, TxnManager, and TxnContextProvider abstractions, then trace transaction status access through sessionVars.TxnCtx and tikvTxn. The issue specifically calls for standardizing forUpdateTS access and adding a read-only status interface, but its remaining TODOs do not define the files, tests, or complete acceptance criteria. Done would require a decided scope and tests covering the unified transaction-status behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.