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.

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

Change common unhandled packet error message to clarify origin #4547

Closed
wants to merge 1 commit into from

Conversation

sjmudd
Copy link
Contributor

@sjmudd sjmudd commented Jan 24, 2019

conn.go has 3 identical error messages for different situations so it's harder to identify where an error has been triggered. This change simply identifies the part of the code affected for easier analysis.

I basically saw this and without protocol knowledge it's not clear which part of the code is triggering the error:

E0124 12:51:40.010442     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 68 ...]
E0124 12:51:40.012055     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 72 ...]
E0124 12:51:40.012139     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 72 ...]
E0124 12:51:40.012210     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 82 ...]
E0124 12:51:40.012270     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 82 ...]
E0124 12:51:40.012329     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 85 ...]
E0124 12:51:40.012386     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 85 ...]
E0124 12:51:40.012450     838 conn.go:801] Got unhandled packet from client 478 (a.b.c.d:58121), returning error: [4 100 ...]

@sjmudd
Copy link
Contributor Author

sjmudd commented Jan 24, 2019

Oddly enough this came I think from a 8.0.13 mysql client. Which is odd as https://dev.mysql.com/doc/dev/mysql-server/8.0.11/page_protocol_com_field_list.html indicates this is COM_FIELD_LIST and is a deprecated command as of 5.7.X. So odd the client is still using it.

@dweitzman
Copy link
Member

Was this a vtgate version older than December 22nd? #4486 fixed a bug in com_set_option which wasn't mentioned in the PR description. There was a missing call to recycleReadPacket

@sjmudd
Copy link
Contributor Author

sjmudd commented Jan 24, 2019

I think the code I'm using is newer. The code I'm running is 6cf46db so not sure.
Either way it's good to make the debug logging a bit tidier, and maybe if byte 0 is the packet type try to label that better than we do now so it's more obvious but I'm not 100% sure if this is always the case as not looked much at the protocol.

@sjmudd
Copy link
Contributor Author

sjmudd commented Jan 24, 2019

I messed up something trying to fix the sign-off so have done a new PR. Bit stupid, sorry. so closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants