Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

Make sure the session param takes effect #280

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

july2993
Copy link
Contributor

@july2993 july2993 commented Mar 10, 2020

What problem does this PR solve?

Make sure the session param takes effect.

What is changed and how it works?

Set session var for every new conn. For every new connection created by the driver, it will set the params first.

Note the previous setSessionConcurrencyVars will set one connection
from the db connection pool, so we can't make sure the session will take
effect later using the sql.DB.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
    check tidb log for the session params setted using the dsn.

Side effects

Related changes

  • Need to cherry-pick to the release branch

  • Need to be included in the release note

Note the previous `setSessionConcurrencyVars` will set one connection
from the db connection pool, so we can't make sure the session will take
affect later using the `sql.DB`.
@july2993 july2993 added the status/PTAL This PR is ready for review. Add this label back after committing new changes label Mar 10, 2020
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@july2993 july2993 added status/LGT1 One reviewer already commented LGTM (LGTM1) and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Mar 10, 2020
Copy link
Collaborator

@IANTHEREAL IANTHEREAL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IANTHEREAL IANTHEREAL merged commit 093ce9f into pingcap:master Mar 10, 2020
@july2993 july2993 deleted the session branch March 10, 2020 09:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/LGT1 One reviewer already commented LGTM (LGTM1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants