[MDEV-21612] Remove COM_MULTI code from server Created: 2020-01-30  Updated: 2020-09-22  Resolved: 2020-09-02

Status: Closed
Project: MariaDB Server
Component/s: Server
Fix Version/s: 10.6.0

Type: Task Priority: Major
Reporter: Vladislav Vaintroub Assignee: Oleksandr Byelkin
Resolution: Fixed Votes: 0
Labels: None

Attachments: File MDEV-21612.url    
Issue Links:
Relates
relates to MDEV-10169 Optimize INSERT for COM_MULTI batch Closed
relates to MDEV-16308 Out of sync with server Closed

 Description   

The COM_MULTI idea did not fly well with connectors.
I'm not aware of plans to use it on connectors, or to develop anything further with it.

Indeed, everything it can do, is possible without changing the protocol, using pipelining techique that JDBC is already successfully deploying. Or the old good JDBC batching,

The possible (and frankly, tiny) advantage of optimiting TCP traffic, in a manner similar to TCP_DELAY, can already be done using tcp_nodelay=0 setting.

Yet the server code is essentially dead compiled, and deployed. It insignificantly slows down the execution of dispatch_command. by adding extra checks "is_next_command", is_com_multi, is_skip_flush". Also from readability/maintainability point, it would be good to remove the extra (and sometimes tricky, like recursve execution of dispatch_command) code.

It could be source of some very subtle bugs . MDEV-16308, protocol breakage that affects .NET connector, is possibly caused by COM_MULTI code (proof pending)

 Comments   
Comment by Bradley Grainger [ 2020-02-04 ]

ADO.NET is introducing a provider-independent batch API: https://github.com/dotnet/runtime/issues/28633

MySqlConnector (OSS .NET library for MySql/MariaDB) implemented the proposed new API using COM_MULTI: https://github.com/mysql-net/MySqlConnector/issues/650

There was a measurable performance improvement with MariaDB: https://gist.github.com/bgrainger/0504a52065eeade9e4b30e88b6363f2c (and this was using a local Docker container so network latency was minimised).

This probably could be simulated by the client using pipelining; however, there's no general-purpose way for a server to indicate that it supports pipelining (perhaps this could be added as a new server flag?). There are many different backends that support the MySQL protocol and AWS Aurora is one example that's broken with pipelining: https://github.com/mysql-net/MySqlConnector/issues/486#issuecomment-385551966

This makes it hard to add as a client-side optimisation that's on by default.

Comment by Bradley Grainger [ 2020-02-04 ]

MySqlConnector performs feature detection, so if a future version of MariaDB no longer advertises the MARIADB_CLIENT_COM_MULTI server capability, it will fall back gracefully.

Comment by Vladislav Vaintroub [ 2020-02-04 ]

bgrainger : the thing with Aurora is not that it does not support pipelining . The thing is that they discard client input . This is a bug, rather than failing to provide a feature. It is better to detect buggy Aurora, and support pipelining, rather than rely on COM_MULTI feature for which you'll the first and only tester.

Comment by Vladislav Vaintroub [ 2020-02-04 ]

Ah, and our JDBC does do pipelining, by default. If you know ways to file bugs with Aurora, please do, or do tell us, as we've no idea.

Anyway, our (mariadb) backend does support pipelining, and it is tested by our JDBC tests. MariaDB can be detected by "-mariadb" in version string. In theory, a proxy can break pipelining, but we'd know by now, like we know about Aurora.

Comment by Bradley Grainger [ 2020-02-04 ]

I do not have any channels to report Aurora bugs (but have accumulated a handful that I would like to report).

Comment by Vladislav Vaintroub [ 2020-02-09 ]

To the backends discarding user packets after the first one, i.e Aurora

I'm quite sure that our protocol requires some pipelining capabilities, because there is a packet type that does not expect any reply from the server
https://dev.mysql.com/doc/internals/en/com-stmt-close.html .

That means, any number of COM_STMT_CLOSE can come from any prepared-statement-capable client (whether it uses pipelining knowingly or not), if it decides to close multiple prepared statements. Or of course there might be
COM_QUERY following COM_STMT_CLOSE. So, any client can send any number multiple packets without waiting for server reply. Thus discarding user input after COM_STMT_CLOSE would be fatal in general case.

Comment by Vladislav Vaintroub [ 2020-04-23 ]

@sanja, could you take a look ? https://github.com/MariaDB/server/commit/035f2b7d0a7865b1815511b387228db789fabfdb

Comment by Vladislav Vaintroub [ 2020-09-02 ]

aparently, it is already done

Comment by Oleksandr Byelkin [ 2020-09-02 ]

wlad did you pushed without review?

Comment by Vladislav Vaintroub [ 2020-09-02 ]

yep, this was pushed with some other patch, from my bb-10.6-wlad .
Sorry, happened. But this was already in review status for a long time.

Generated at Thu Feb 08 09:08:28 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.