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

server: close connection when tidb server closed. #8446

Merged
merged 5 commits into from Nov 27, 2018

Conversation

@crazycs520
Contributor

crazycs520 commented Nov 26, 2018

What problem does this PR solve?

If tidb is not gracefully shutdown, tidb should kill connection to avoid continue processing the connection request. Because after ddl.Stop(), the schema version maybe outdate, processing request now may cause some data inconsistency problem.

What is changed and how it works?

Kill all connection when server close if server is not gracefully shutdown.

Check List

Tests

  • Unit test, add test later... Actually, I am thinking how to add test, If anyone know, please tell me. Thank you very much. 😂 😂 😂

Code changes

  • Has exported function/method change

Side effects

Related changes

  • Need to cherry-pick to the release branch

This change is Reviewable

@@ -365,6 +369,17 @@ func (s *Server) Kill(connectionID uint64, query bool) {
}
}
// KillAllConnections kills all connections when server is not gracefully shutdown.
func (s *Server) KillAllConnections() {

This comment has been minimized.

@winkyao

winkyao Nov 26, 2018

Member

Call cancel function in clientConn.dispatch can make the connection return?

This comment has been minimized.

@crazycs520

crazycs520 Nov 26, 2018

Contributor

Call ctx.cancel will cancel the current execution, atomic.StoreInt32(&conn.status, connStatusWaitShutdown) will make the connection close itself later.

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 26, 2018

need another LGTM

@tiancaiamao

This comment has been minimized.

Contributor

tiancaiamao commented Nov 26, 2018

LGTM

Through I'm afraid the problem can't be solved by those changes:

Because after ddl.Stop(), the schema version maybe outdate, processing request now may cause some data inconsistency problem.

killConn maybe can't stop the inflight query from been processed

@tiancaiamao

This comment has been minimized.

Contributor

tiancaiamao commented Nov 26, 2018

/run-all-tests

@crazycs520 crazycs520 force-pushed the crazycs520:close-conn branch from af5f403 to 6d74762 Nov 26, 2018

@crazycs520

This comment has been minimized.

Contributor

crazycs520 commented Nov 26, 2018

/run-all-tests

@lysu

This comment has been minimized.

Member

lysu commented Nov 26, 2018

so we need wait s.clients be 0 before call domain.Close in best situation??(not affect lgtm) 🤣

Show resolved Hide resolved server/server.go Outdated
@crazycs520

This comment has been minimized.

Contributor

crazycs520 commented Nov 26, 2018

/run-all-tests

@tiancaiamao

This comment has been minimized.

Contributor

tiancaiamao commented Nov 26, 2018

LGTM

@crazycs520

This comment has been minimized.

Contributor

crazycs520 commented Nov 27, 2018

/run-common-test tidb-test=pr/668

@crazycs520

This comment has been minimized.

Contributor

crazycs520 commented Nov 27, 2018

/run-integration-common-test tidb-test=pr/668

@tiancaiamao tiancaiamao merged commit 654964a into pingcap:master Nov 27, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment