session: make `TxnInfo()` return even if process info is empty (#57044) by ti-chi-bot · Pull Request #57164 · pingcap/tidb

ti-chi-bot · 2024-11-06T06:30:43Z

This is an automated cherry-pick of #57044

What problem does this PR solve?

Issue Number: close #57043

Problem Summary:

Lines 4026 to 4030 in c240fdf

    
           txnInfo := tmp.TxnInfo() 
        
           if txnInfo != nil { 
        
           	startTS = txnInfo.StartTS 
        
           	processInfoID = txnInfo.ConnectionID 
        
           }

TxnInfo() could be nil if process info is not set(see the implementation of TxnInfo()). Such sessions are commonly used in DDL/DXF:

tidb/pkg/ddl/backfilling_operators.go

Lines 533 to 539 in c240fdf

    
           sessCtx, err := w.sessPool.Get() 
        
           if err != nil { 
        
           	logutil.Logger(w.ctx).Error("tableScanWorker get session from pool failed", zap.Error(err)) 
        
           	w.ctx.onError(err) 
        
           	return 
        
           } 
        
           w.se = session.NewSession(sessCtx)

And they only invoke session.Begin() before using:

tidb/pkg/ddl/index_cop.go

Lines 43 to 48 in c240fdf

    
           func wrapInBeginRollback(se *sess.Session, f func(startTS uint64) error) error { 
        
           	err := se.Begin(context.Background()) 
        
           	if err != nil { 
        
           		return errors.Trace(err) 
        
           	} 
        
           	defer se.Rollback()

As a result, the startTS from internal DDL sessions will not be reported to etcd, and GC safepoint will be advanced unexpectedly.

What changed and how does it work?

Make TxnInfo() return even if process info is empty.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

tangenta · 2024-11-12T08:31:17Z

/retest

tangenta · 2024-11-12T08:45:44Z

/retest

codecov · 2024-11-13T04:18:56Z

Codecov Report

Attention: Patch coverage is 90.00000% with 5 lines in your changes missing coverage. Please review.

Please upload report for BASE (release-7.5@7e13648). Learn more about missing BASE report.

Additional details and impacted files

@@               Coverage Diff                @@
##             release-7.5     #57164   +/-   ##
================================================
  Coverage               ?   72.1557%           
================================================
  Files                  ?       1415           
  Lines                  ?     412652           
  Branches               ?          0           
================================================
  Hits                   ?     297752           
  Misses                 ?      94934           
  Partials               ?      19966

Flag	Coverage Δ
unit	`72.1557% <90.0000%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.9400% <0.0000%> (?)`
parser	`∅ <0.0000%> (?)`
br	`53.5535% <0.0000%> (?)`

ti-chi-bot · 2024-11-13T06:49:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lcwangchao, tangenta

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lcwangchao,tangenta]
~~pkg/ddl/OWNERS~~ [tangenta]
~~pkg/infoschema/OWNERS~~ [tangenta]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-11-13T06:49:39Z

[LGTM Timeline notifier]

Timeline:

2024-11-13 06:48:32.079784398 +0000 UTC m=+425274.270653387: ☑️ agreed by tangenta.
2024-11-13 06:49:38.035908859 +0000 UTC m=+425340.226777855: ☑️ agreed by lcwangchao.

…ap#57044) (pingcap#57164) close pingcap#57043

This is an automated cherry-pick of pingcap#57044

bb692bc

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/cherry-pick-for-release-7.5 This PR is cherry-picked to release-7.5 from a source PR. labels Nov 6, 2024

ti-chi-bot mentioned this pull request Nov 6, 2024

session: make TxnInfo() return even if process info is empty #57044

Merged

13 tasks

ti-chi-bot assigned tangenta Nov 6, 2024

ti-chi-bot bot added the do-not-merge/cherry-pick-not-approved label Nov 6, 2024

tangenta added 3 commits November 12, 2024 15:19

resolve conflicts

14bc88c

fix build and test

0825b5d

update bazel

0d77ff9

upgrade failpoint-ctl

c24cb06

ti-chi-bot bot added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Nov 13, 2024

tangenta approved these changes Nov 13, 2024

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Nov 13, 2024

lcwangchao approved these changes Nov 13, 2024

View reviewed changes

ti-chi-bot bot added the lgtm label Nov 13, 2024

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Nov 13, 2024

ti-chi-bot bot merged commit 7662c96 into pingcap:release-7.5 Nov 13, 2024

tangenta pushed a commit to tangenta/tidb that referenced this pull request Dec 30, 2024

session: make TxnInfo() return even if process info is empty (pingc…

0bb9036

…ap#57044) (pingcap#57164) close pingcap#57043

tangenta pushed a commit to tangenta/tidb that referenced this pull request Jan 7, 2025

session: make TxnInfo() return even if process info is empty (pingc…

7fd1656

…ap#57044) (pingcap#57164) close pingcap#57043

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

session: make `TxnInfo()` return even if process info is empty (#57044)#57164

session: make `TxnInfo()` return even if process info is empty (#57044)#57164
ti-chi-bot[bot] merged 5 commits intopingcap:release-7.5from
ti-chi-bot:cherry-pick-57044-to-release-7.5

ti-chi-bot commented Nov 6, 2024

Uh oh!

tangenta commented Nov 12, 2024

Uh oh!

tangenta commented Nov 12, 2024

Uh oh!

codecov bot commented Nov 13, 2024 •

edited

Loading

Uh oh!

ti-chi-bot bot commented Nov 13, 2024

Uh oh!

ti-chi-bot bot commented Nov 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	txnInfo := tmp.TxnInfo()
	if txnInfo != nil {
	startTS = txnInfo.StartTS
	processInfoID = txnInfo.ConnectionID
	}

	sessCtx, err := w.sessPool.Get()
	if err != nil {
	logutil.Logger(w.ctx).Error("tableScanWorker get session from pool failed", zap.Error(err))
	w.ctx.onError(err)
	return
	}
	w.se = session.NewSession(sessCtx)

	func wrapInBeginRollback(se *sess.Session, f func(startTS uint64) error) error {
	err := se.Begin(context.Background())
	if err != nil {
	return errors.Trace(err)
	}
	defer se.Rollback()

Conversation

ti-chi-bot commented Nov 6, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tangenta commented Nov 12, 2024

Uh oh!

tangenta commented Nov 12, 2024

Uh oh!

codecov bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ti-chi-bot bot commented Nov 13, 2024

Uh oh!

ti-chi-bot bot commented Nov 13, 2024

[LGTM Timeline notifier]

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Nov 13, 2024 •

edited

Loading