Add support for MySQL pipelining execution #1168

BillyYccc · 2022-03-28T15:54:42Z

Motivation:

fixes #861, fixes #873

Changes:

add support for query pipelining execution
mysql connection now handles the fire & forget command in the queue order correctly
rework the batch execution to use pipelining mode by default

Conformance:

You should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md
Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines

Signed-off-by: Billy Yuan <[email protected]>

…execution command being sent Signed-off-by: Billy Yuan <[email protected]>

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc · 2022-03-28T16:09:53Z

This feature is now ready for reviewing

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/MySQLBatchException.java

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/MySQLDecoder.java

vietj

LGTM overall, a couple of minor changes to do and we are good to go.

tsegismont

Thank you @BillyYccc ! I just have a question about usage

tsegismont · 2022-04-01T08:35:21Z

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/MySQLBatchException.java

+ * {@code MySQLBatchException} is thrown if an error occurs during executions when using {@link io.vertx.sqlclient.PreparedQuery#executeBatch(List)}.
+ * The client will try to execute with all the params no matter if one iteration of the executions fails, the iteration count is calculated from zero.
+ */
+public class MySQLBatchException extends RuntimeException {


Can you think about a way to combine this type with MySQLException in a hierarchy? It can be convenient for users to identity where the issue comes from when the exception bubbles up to upper layers.

how about using a mapping like Map<Integer, MySQLException> to represent the errors in a batching process? I think MySQL needs this because it emulates batching by sending multiple requests and might receive multiple error responses which does not behave like Postgres client batching(one request & one response)

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc · 2022-04-01T12:57:07Z

I also need to label this as breaking change because it changes the way how the batching works.

vietj · 2022-04-04T09:02:05Z

@BillyYccc can you update the breaking change wiki page too https://github.com/vert-x3/wiki/wiki/4.3.0-Deprecations-and-breaking-changes

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc · 2022-04-04T14:55:08Z

@BillyYccc can you update the breaking change wiki page too https://github.com/vert-x3/wiki/wiki/4.3.0-Deprecations-and-breaking-changes

done

vietj · 2022-04-07T06:42:54Z

Another small change, I think receiveNoResponsePacket method should be renamed expectNoResponsePacket

vietj · 2022-04-11T14:02:51Z

ping @BillyYccc

vertx-mysql-client/src/test/java/io/vertx/mysqlclient/MySQLPipeliningTest.java

…client Signed-off-by: Billy Yuan <[email protected]>

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc · 2022-05-07T14:17:10Z

I think it's ready for a final review now

vietj · 2022-05-09T06:03:25Z

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/MySQLConnectOptions.java

@@ -65,6 +65,7 @@ public static MySQLConnectOptions fromUri(String connectionUri) throws IllegalAr
  public static final Map<String, String> DEFAULT_CONNECTION_ATTRIBUTES;
  public static final SslMode DEFAULT_SSL_MODE = SslMode.DISABLED;
  public static final String DEFAULT_CHARACTER_ENCODING = "UTF-8";
+  public static final int DEFAULT_PIPELINING_LIMIT = 1;


why not use the same value than PgConnectOptions which is 256 ?

I'm assuming this is for backward compatibility ?

yes, I'm trying to avoid introducing breaking changes.

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/MySQLPoolOptions.java

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/MySQLEncoder.java

...ysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/ExtendedQueryCommandBaseCodec.java

vietj

A few minor change and a question about pipelining limit default = 1 (assuming it's for being conservative and backward compatible).

vietj · 2022-05-09T06:15:56Z

@BillyYccc if you can handle the minor changes soon, it can be merged and be part of Vert.x 4.3 release

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc · 2022-05-09T13:26:32Z

all comments should be addressed now

vietj · 2022-05-09T13:47:47Z

thanks for your great contribution @BillyYccc !

BillyYccc · 2022-05-09T13:50:50Z

thanks for the reviewing 😃

BillyYccc added 7 commits March 28, 2022 23:23

initial work

df387a1

Signed-off-by: Billy Yuan <[email protected]>

MySQL fire-and-forget commands should be handled in order

1c42d9a

Signed-off-by: Billy Yuan <[email protected]>

managed prepared statement closing command should be sent as soon as …

14a9a7f

…execution command being sent Signed-off-by: Billy Yuan <[email protected]>

rework the MySQL batch execution to use pipeling mode by default

7183c91

Signed-off-by: Billy Yuan <[email protected]>

minors for the MySQL pipelining javadoc

cf334e0

Signed-off-by: Billy Yuan <[email protected]>

update to the newest code

c8519e0

Signed-off-by: Billy Yuan <[email protected]>

minors

511b288

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc added enhancement module:mysql labels Mar 28, 2022

BillyYccc requested review from vietj and tsegismont March 28, 2022 16:08

vietj added this to the 4.3.0 milestone Apr 1, 2022

vietj reviewed Apr 1, 2022

View reviewed changes

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/MySQLBatchException.java Outdated Show resolved Hide resolved

vietj reviewed Apr 1, 2022

View reviewed changes

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/MySQLDecoder.java Outdated Show resolved Hide resolved

vietj requested changes Apr 1, 2022

View reviewed changes

tsegismont reviewed Apr 1, 2022

View reviewed changes

BillyYccc added 2 commits April 1, 2022 19:52

refactor the repeated code

7f77b3e

Signed-off-by: Billy Yuan <[email protected]>

improve the MySQLBatchException javadoc

bc4bd11

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc added the breaking change label Apr 1, 2022

vietj mentioned this pull request Apr 4, 2022

Vert.x 4 Roadmap vert-x3/issues#566

Open

update the mysql batching exception field

5c95c9d

Signed-off-by: Billy Yuan <[email protected]>

BillyYccc added 2 commits May 1, 2022 19:13

rename according to reviews

a7c429f

rework pipelining tests

99d61a1

BillyYccc commented May 1, 2022

View reviewed changes

vertx-mysql-client/src/test/java/io/vertx/mysqlclient/MySQLPipeliningTest.java Outdated Show resolved Hide resolved

BillyYccc added 2 commits May 4, 2022 01:28

move mysql pipelining query tests to TCK and enable them in postgres …

3204c0c

…client Signed-off-by: Billy Yuan <[email protected]>

fix incorrect testcases

a70e6c8

Signed-off-by: Billy Yuan <[email protected]>

vietj reviewed May 9, 2022

View reviewed changes

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/MySQLPoolOptions.java Outdated Show resolved Hide resolved

vietj reviewed May 9, 2022

View reviewed changes

vertx-mysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/MySQLEncoder.java Outdated Show resolved Hide resolved

vietj reviewed May 9, 2022

View reviewed changes

...ysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/ExtendedQueryCommandBaseCodec.java Outdated Show resolved Hide resolved

vietj reviewed May 9, 2022

View reviewed changes

...ysql-client/src/main/java/io/vertx/mysqlclient/impl/codec/ExtendedQueryCommandBaseCodec.java Outdated Show resolved Hide resolved

vietj approved these changes May 9, 2022

View reviewed changes

BillyYccc added 3 commits May 9, 2022 20:54

fix license header

31e0aff

Signed-off-by: Billy Yuan <[email protected]>

rename methods

324d5a9

Signed-off-by: Billy Yuan <[email protected]>

optimize using shift operator

f71739f

Signed-off-by: Billy Yuan <[email protected]>

vietj merged commit bef0b4b into eclipse-vertx:master May 9, 2022

tsegismont mentioned this pull request Aug 23, 2022

Quarkus version change from 2.10.1.Final to 2.11.1.Final has breaking changes with Percona SqlProxy quarkusio/quarkus#27386

Closed

tsegismont mentioned this pull request Sep 16, 2022

MySQL Client sends close statement commend before all rows are decoded #1234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for MySQL pipelining execution #1168

Add support for MySQL pipelining execution #1168

BillyYccc commented Mar 28, 2022

BillyYccc commented Mar 28, 2022

vietj left a comment

tsegismont left a comment

tsegismont Apr 1, 2022

BillyYccc Apr 1, 2022

BillyYccc commented Apr 1, 2022

vietj commented Apr 4, 2022

BillyYccc commented Apr 4, 2022

vietj commented Apr 7, 2022

vietj commented Apr 11, 2022

BillyYccc commented May 7, 2022

vietj May 9, 2022

vietj May 9, 2022

BillyYccc May 9, 2022

vietj left a comment

vietj commented May 9, 2022

BillyYccc commented May 9, 2022

vietj commented May 9, 2022

BillyYccc commented May 9, 2022

Add support for MySQL pipelining execution #1168

Add support for MySQL pipelining execution #1168

Conversation

BillyYccc commented Mar 28, 2022

BillyYccc commented Mar 28, 2022

vietj left a comment

Choose a reason for hiding this comment

tsegismont left a comment

Choose a reason for hiding this comment

tsegismont Apr 1, 2022

Choose a reason for hiding this comment

BillyYccc Apr 1, 2022

Choose a reason for hiding this comment

BillyYccc commented Apr 1, 2022

vietj commented Apr 4, 2022

BillyYccc commented Apr 4, 2022

vietj commented Apr 7, 2022

vietj commented Apr 11, 2022

BillyYccc commented May 7, 2022

vietj May 9, 2022

Choose a reason for hiding this comment

vietj May 9, 2022

Choose a reason for hiding this comment

BillyYccc May 9, 2022

Choose a reason for hiding this comment

vietj left a comment

Choose a reason for hiding this comment

vietj commented May 9, 2022

BillyYccc commented May 9, 2022

vietj commented May 9, 2022

BillyYccc commented May 9, 2022