-
Notifications
You must be signed in to change notification settings - Fork 9
[Draft] Control vtgate buffer polling with mysql_conn_buffer_polling parameter #254
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good sans a few nits! going to take this branch and default the pooling to on so we can do some testing!
@@ -183,31 +186,32 @@ type Listener struct { | |||
PreHandleFunc func(context.Context, net.Conn, uint32) (net.Conn, error) | |||
} | |||
|
|||
// NewFromListener creates a new mysql listener from an existing net.Listener | |||
func NewFromListener(l net.Listener, authServer AuthServer, handler Handler, connReadTimeout time.Duration, connWriteTimeout time.Duration) (*Listener, error) { | |||
// NewFromListener creares a new mysql listener from an existing net.Listener |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would revert this bit
//go:build gofuzz | ||
// +build gofuzz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unclear what this change is about. should we revert it?
@@ -13,7 +16,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ | |||
// +build gofuzz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment as above.
c.bufferedReader.Reset(nil) | ||
readersPool.Put(c.bufferedReader) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be worth checking that the bufferedReader
isn't nil before doing this.
@@ -72,6 +72,11 @@ var ( | |||
mysqlConnWriteTimeout = flag.Duration("mysql_server_write_timeout", 0, "connection write timeout") | |||
mysqlQueryTimeout = flag.Duration("mysql_server_query_timeout", 0, "mysql query timeout") | |||
|
|||
// TODO: enable this once the loadtest is done | |||
// mysqlConnBufferPooling = flag.Bool("mysql_conn_buffer_polling", false, "If set, vtgate will pool connection buffers") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit for later: would rename flag to have consistent prefix with other flags in this file. something like mysql_server_read_conn_pooling
? would also update flag description to specify that it's connection reading as connection writing is already pooled.
Description
Adding an option for the vtgate to pool the connection buffer instead of creating new buffer for each new connection.
Related Issue(s)
Slack vtgate doses