Skip to content
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.

Sign up for GitHub

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

Jump to bottom

server: filter the EOF error for normal closed at handshake #31081

Merged
merged 7 commits into from
Dec 31, 2021
Merged

server: filter the EOF error for normal closed at handshake #31081

merged 7 commits into from
Dec 31, 2021

Conversation

knull-cn
Copy link
Contributor

@knull-cn knull-cn commented Dec 28, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: close #31063

Problem Summary: loader balance (eg: gcp lb )check health : construct tcp link ok and closed before handshake , tidb will print error log for EOF.

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
    • run tcp client -- remember change the ip/port of tidb
      • tcp client test code was show latter
    • at tidb.log , you will see the EOF at debug level log and see other errors info in error level log
  • No code
# `tcp client` test code
#!/usr/bin/env python    
import socket    
    
TCP_IP = '127.0.0.1'    
TCP_PORT = 4000    
BUFFER_SIZE = 1024    
MESSAGE = ""    
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)    
s.connect((TCP_IP, TCP_PORT))    
s.close()

Release note

None

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 28, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • djshow832
  • jackysp

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 submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 28, 2021
@sre-bot
Copy link
Contributor

sre-bot commented Dec 28, 2021

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@sre-bot
Copy link
Contributor

sre-bot commented Dec 28, 2021 •
edited
Loading

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/0c89d9e6023392ffbe93945e2d83d5f5a0a58b06

@bb7133
Copy link
Member

bb7133 commented Dec 29, 2021

Please follow PR Title Format:

pkg [, pkg2, pkg3]: what is changed
Or if the count of mainly changed packages are more than 3, use

*: what is changed
After you have format title, you can leave a comment /run-check_title to recheck it

@knull-cn Please fix the title check.

@bb7133
Copy link
Member

bb7133 commented Dec 29, 2021

/run-check_dev_2

bb7133
bb7133 reviewed Dec 29, 2021
View reviewed changes
server/server.go Outdated Show resolved Hide resolved
@bb7133
Copy link
Member

bb7133 commented Dec 29, 2021

Please add a test case for your change, thanks!

@sre-bot
Copy link
Contributor

sre-bot commented Dec 29, 2021

Please follow PR Title Format:

  • pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

  • *: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

@knull-cn knull-cn changed the title fix the error log ,filter the EOF error for normal closed. server : fix the error log ,filter the EOF error for normal closed. Dec 29, 2021
@knull-cn knull-cn changed the title server : fix the error log ,filter the EOF error for normal closed. server: fix the error log ,filter the EOF error for normal closed. Dec 29, 2021
@knull-cn knull-cn changed the title server: fix the error log ,filter the EOF error for normal closed. server: fix the error log ,filter the EOF error for normal closed Dec 29, 2021
@knull-cn knull-cn changed the title server: fix the error log ,filter the EOF error for normal closed server: filter the EOF error for normal closed at handshake Dec 29, 2021
@knull-cn
Copy link
Contributor Author

/run-check_title

@knull-cn
Copy link
Contributor Author

/run-unit-test

@jackysp jackysp added the type/enhancement The issue or PR belongs to an enhancement. label Dec 29, 2021
jackysp
jackysp reviewed Dec 29, 2021
View reviewed changes
server/server.go Outdated Show resolved Hide resolved
@knull-cn
Copy link
Contributor Author

/run-check_dev_2

1 similar comment
@knull-cn
Copy link
Contributor Author

/run-check_dev_2

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 30, 2021
@djshow832
Copy link
Contributor

Please add a test case for your change, thanks!

I think it's fine to only add a manual test because it only touches the logs.

@lichunzhu
Copy link
Contributor

/run-mysql-test

@knull-cn
Copy link
Contributor Author

/run-mysql-test

jackysp
jackysp approved these changes Dec 31, 2021
View reviewed changes
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 31, 2021
@jackysp
Copy link
Member

jackysp commented Dec 31, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: b6373e0

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 31, 2021
@knull-cn
Copy link
Contributor Author

/run-check_dev_2

@knull-cn
Copy link
Contributor Author

/run_unit-test

@knull-cn
Copy link
Contributor Author

/run-unit-test

@knull-cn
Copy link
Contributor Author

/run-check_dev2

@knull-cn
Copy link
Contributor Author

/run-mysql-test

@ti-chi-bot ti-chi-bot merged commit 45879b5 into pingcap:master Dec 31, 2021
rebelice pushed a commit to TiInterstellar/tidb that referenced this pull request Jan 5, 2022
@crazycs520 @xhebox
@guo-shaoge @knull-cn @mengxin9014
hello world (#2)
5ab74c9 
* topsql: make topsql enable only be controlled by pub/sub sink (pingcap#31209)

* ddl: support batch create table  (pingcap#28763)

* executor: fix data race in IndexMergeReaderExec (pingcap#31230)

close pingcap#31229

* server: filter the EOF error for normal closed at handshake  (pingcap#31081)

close pingcap#31063

* expression: change date add function return type (pingcap#28133)

close pingcap#27573

* support create interval partition

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support create interval partition (support int/timestamp partition key)

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* parser: support alter table partitions move engine statement

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support ddl operation

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support interval partition manager

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support interval partition manager handle job framwork

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support auto create interval partition when insert meet no partition suitable error

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* fix bug

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* fix cancel job and load old job then continue to do

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* make partition readonly work(not allow to insert/update/delete)

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* add begin,end time in tables

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* tiny fix for auto create interval partition in concurrent case

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* init

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* init

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* todo: remove flag

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* fix dumpling

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* remove data in aws s3 when drop/truncate table/partition

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* make hello world work

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* remove debug info

Signed-off-by: crazycs520 <crazycs520@gmail.com>

Co-authored-by: xhe <xw897002528@gmail.com>
Co-authored-by: guo-shaoge <shaoge1994@163.com>
Co-authored-by: knull-cn <hu__haifeng@163.com>
Co-authored-by: Meng Xin <tregoldmeng@gmail.com>
@DanielZhangQD DanielZhangQD mentioned this pull request Jan 7, 2022
Closed
@knull-cn knull-cn deleted the kdev branch February 22, 2022 01:58
@jackysp jackysp added the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Mar 30, 2022
@ti-srebot ti-srebot mentioned this pull request Mar 30, 2022
Open
4 tasks
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Mar 30, 2022
@knull-cn @ti-srebot
cherry pick pingcap#31081 to release-5.4
697fe45 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-5.4 in PR #33589

elsa0520 pushed a commit to elsa0520/tidb that referenced this pull request Sep 11, 2023
@knull-cn @elsa0520
server: filter the EOF error for normal closed at handshake (pingcap#…
4a4e7e9 
…31081)

close pingcap#31063
@elsa0520 elsa0520 mentioned this pull request Sep 11, 2023
Merged
12 tasks
ti-chi-bot bot pushed a commit that referenced this pull request Sep 11, 2023
@elsa0520
server: filter the EOF error for normal closed at handshake (#31081) (#…
783639f 
…46866)

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

@bb7133 bb7133 bb7133 left review comments

@djshow832 djshow832 djshow832 approved these changes

@jackysp jackysp jackysp approved these changes

Assignees
No one assigned
Labels
needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

server: do not report ["encountered error"] [error=EOF] for TCPConn alive detectors
8 participants
@knull-cn @ti-chi-bot @sre-bot @bb7133 @djshow832 @lichunzhu @jackysp @ti-srebot