New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

domain,session: simplify the session pool of domain #8456

Merged
merged 6 commits into from Nov 27, 2018

Conversation

Projects
None yet
4 participants
@tiancaiamao
Contributor

tiancaiamao commented Nov 26, 2018

What problem does this PR solve?

Fix the bug that TiDB may hang a long time even after it get the exit signal.

DDL or stats may take sessions from the session pool to ExecRestrictedSQL for a long time,
while the old pool will block waiting until the session to put back to the pool.

What is changed and how it works?

The session pool now just serve as a cache, close the pool will no longer
wait for all resources to be put back to the pool.

Check List

Tests

  • No code (actually a refactor, no new code)

PTAL @winkyao @zimulala


This change is Reviewable

tiancaiamao added some commits Nov 26, 2018

domain,session: simplify the session pool of domain
The session pool now just serve as a cache, close the pool will no longer
wait for all resources to be put back to the pool.
Fix the bug that TiDB may hang a long time even after it get the exit signal
@tiancaiamao

This comment has been minimized.

Contributor

tiancaiamao commented Nov 27, 2018

}
}
func (p *sessionPool) Get() (resource pools.Resource, err error) {

This comment has been minimized.

@zz-jason

zz-jason Nov 27, 2018

Member

should we check p.mu.closed?

This comment has been minimized.

@tiancaiamao

tiancaiamao Nov 27, 2018

Contributor

Not necessary.
line 629 handles the sessionPool is closed scene.

Show resolved Hide resolved domain/domain.go
Show resolved Hide resolved domain/domain.go
@lysu

This comment has been minimized.

Member

lysu commented Nov 27, 2018

LGTM

@lysu lysu added the status/LGT1 label Nov 27, 2018

@winkyao

LGTM

@zz-jason

Reviewed 2 of 3 files at r1, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lysu)

@lysu

lysu approved these changes Nov 27, 2018

:lgtm:

Reviewed 3 of 3 files at r1, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lysu)

@zz-jason

:lgtm:

Reviewed 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@zz-jason zz-jason merged commit cadab30 into pingcap:master Nov 27, 2018

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
code-review/reviewable 3 files reviewed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@tiancaiamao tiancaiamao deleted the tiancaiamao:session-pool branch Nov 27, 2018

tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Nov 27, 2018

tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Nov 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment