session: test that temporary tables will also be retried

[djshow832]

What problem does this PR solve?

Close #24560

Problem Summary:
Even if data of global temporary tables are only visible to the current transaction, DML on it needs to be retried because DML on normal tables may rely on the data of global temporary tables.

E.g.

INSERT INTO normal_table SELECT * FROM temp_table;
UPDATE normal_table, temp_table SET normal_table.b=temp_table.b WHERE normal_table.a=temp_table.a;

What is changed and how it works?

What's Changed: Add some test cases.

Related changes

• N/A

Check List

Tests

• Unit test

Side effects

• N/A

Release note

• No release notes.

[xhebox]

/lgtm

[tiancaiamao]

/LGTM

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• tiancaiamao
• xhebox

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

[tiancaiamao]

/merge

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[ti-chi-bot]

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

[djshow832]

/merge cancel

[lysu]

it seems the race is caused by

tidb/session/session.go

Lines 530 to 550 in b1d134d

	// Filter out the temporary table key-values.
	if tables := s.sessionVars.TxnCtx.GlobalTemporaryTables; tables != nil {
	memBuffer := s.txn.GetMemBuffer()
	for tid := range tables {
	seekKey := tablecodec.EncodeTablePrefix(tid)
	endKey := tablecodec.EncodeTablePrefix(tid + 1)
	iter, err := memBuffer.Iter(seekKey, endKey)
	if err != nil {
	return err
	}
	for iter.Valid() && iter.Key().HasPrefix(seekKey) {
	if err = memBuffer.Delete(iter.Key()); err != nil {
	return errors.Trace(err)
	}
	s.txn.UpdateEntriesCountAndSize()
	if err = iter.Next(); err != nil {
	return errors.Trace(err)
	}
	}
	}
	}

the code snippet breaks the agreement that "memdb stage must be empty after stepped into txn commit phase"

so commit's reset() method will try to rollback and change those stage buffer during async cleanup.

it's better let those filter logic be executed as a virtual stmt and do a StmtCommit, maybe can change it to

	// Filter out the temporary table key-values.
	if tables := s.GetSessionVars().TxnCtx.GlobalTemporaryTables; tables != nil {
		if err := func() error {
			memBuffer := s.txn.GetMemBuffer()
			defer func() {
				s.txn.cleanup() // reset and new a empty stage buffer
			}()
			for tid := range tables {
				seekKey := tablecodec.EncodeTablePrefix(tid)
				endKey := tablecodec.EncodeTablePrefix(tid + 1)
				iter, err := memBuffer.Iter(seekKey, endKey)
				if err != nil {
					return err
				}
				for iter.Valid() && iter.Key().HasPrefix(seekKey) {
					if err = memBuffer.Delete(iter.Key()); err != nil {
						return errors.Trace(err)
					}
					s.txn.UpdateEntriesCountAndSize()
					if err = iter.Next(); err != nil {
						return errors.Trace(err)
					}
				}
			}
			s.txn.flushStmtBuf() // flush to root membuffer
			return nil
		}(); err != nil {
			return err
		}
	}

or just as @cfzjywxk said add a StmtCommit before tikvTxn commit

[tiancaiamao]

memdb stage must be empty

Do you mean that the Delete operation make the stage dirty? @lysu

[lysu]

memdb stage must be empty

Do you mean that the Delete operation make the stage dirty? @lysu

yes~, those delete will make latest stage buffer dirty, so reset will try to rollback dirty buffer and cleanup will try to read dirty buffer at the same time, it's better make those delete op affact on root buffer before commit, so there are no race happen between reset and cleanup (because reset do nothing..) @tiancaiamao

...or maybe we can choose remove defer st.reset(), but it seems need change more things - - @cfzjywxk seems try to do that..

[djshow832]

/run-unit-test

[tiancaiamao]

/LGTM

[lysu]

/lgtm

[djshow832]

/merge

[ti-chi-bot]

This pull request has been accepted and is ready to merge.

Commit hash: dad6d7a

[ti-chi-bot]

@djshow832: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[djshow832]

/run-check_dev_2