Review requested for
https://code.launchpad.net/~massimo-siani/mariadb-java-client/CONJ-115

I edited the description. I had forgot a "not". The correct text should be "I'm not sure about the exact limit, maybe 1 GiB".

I think the suggested fix in https://code.launchpad.net/~massimo-siani/mariadb-java-client/CONJ-115 is acceptable. This will make the maximum LOAD DATA INFILE be 65536 * 8192 * 2 = 1073741824 bytes = 1 GiB. The downside of the fix is that is does not address the Exception text at row 84 of PacketOutputStream.java:

MySQL protocol limit reached, you cannot send more than 4GB of data

I might have changed my mind about if the fix is acceptable. When running my test case using Mysql JDBC (5.1.26) with different values it passed above 1 GiB of data:

60000000  rows: 1297777795 bytes, 196 sec

120000000 rows: 2657777797 bytes, 268 sec

(I also had to change the data type totalNumberOfBytesRead from int to long to be able to count the number of bytes above 2 GiB)

I’m beginning to think the there is no limit or that the limit is very high. I used Mysql JDBC 5.1.26 and 5.5.33a-MariaDB server and ran tests:

 60000000 rows:  1297777795 bytes,  196 sec

120000000 rows:  2657777797 bytes,  268 sec

180000000 rows:  4097777797 bytes,  426 sec

200000000 rows:  4577777797 bytes,  484 sec

300000000 rows:  6977777797 bytes,  710 sec

500000000 rows: 11777777797 bytes, 1192 sec

800000000 rows: 18977777797 bytes, 1952 sec

I used max_allowed_packet=128M. Don’t know if that setting matters.

I stopped running at about 18 GiB because my I was running out of hard drive and time to wait for the test to execute. At 18 GiB of LOAD DATA INFILE my MYD file was 48 GiB. To be able to run larger files I would have to optimize GiganticLoadDataInfileTest.java (probably by adding more or larger columns to get a higher throughput) and perhaps also using a faster SSD.

So If Mysql JDBC has no limit (or a super high limit), shouldn’t MariaDB JDBC also behave the same?

There is no limit, I believe. IIRC, seqno just rolls over from 255 to 0. Massimo, may I suggest NOT to include a unit test with gigabaytes of load data infile? You can still leave the code but at least mark it as @Ignored or whatever was used to ignore the tests.

If there really is no limit I have a simple (a bit naive) solution in the attached file CONJ-115_lennart_simple_solution.patch. My solution is to simply remove the throwing of “MySQL protocol limit”-Exception. My patch also increases the LOAD DATA INFILE send buffer a bit.

As I look around in the code I see few test cases that tests PacketOutputStream directly, so I would be very careful before including my patch in the production code. If I worked on the code at full time I think I would write a JUnit test with a server mock up (that simply reads bytes from the stream and throws Exception if something strange is found).

(The patch is in Git diff format, but can be applied in Eclipse by selecting “Ignore leading path name segments” to 1).

I do not know how you would write a test checking for "no limit". Should Junit, part of unit test suite , part of normal build, fill the disk completely, will this be considered success ? For that type of stuff , one-time test is absolutely enough, on a single developer box IMO.

Yeah, a "no limit" test case can be a bit hard to write. My main concern about lacking tests is to catch regressions, that is other errors that may be caused if my patch is not perfect. For example lets say my patch makes it impossible to send a large INSERT INTO table VALUES()-statement. Or if my code makes the seqNo a non-continues number sequence.

I have tested my patch on a live-ish test system with a lot of traffic. From what I can see it works fine. I had also added a small logger to log the sizes. The largest LOAD DATA INFILE I saw was 1381971038 bytes = 1,3 GiB. So in my case I would require a fix that can use larger than 1 GiB of data.

Why I'm I using so large files? I don't know. I think the code I'm working in has a row limit, but some tables have so many columns that the byte count get very large.

I think the problem is, that we use a fixed buffer of 8192 bytes instead of filling internal buffer (MAX_PACKET_LENGTH), e.g.

    public void sendFile(InputStream is, int seq) throws IOException{

        byte[] buffer = new byte[8192];

        int len;

        startPacket(seq++);

        while((len = is.read(buffer)) > 0) {

          write(buffer, 0, len);

        }

        finishPacket();

        writeEmptyPacket(seq);

    }

Please don't implement a solution like this:

        byte[] buffer = new byte[MAX_PACKET_LENGTH - HEADER_LENGTH];

It won't work for servers that has a smaller max_allowed_packet setting.

I'm not sure this is the best solution, but the code in the CONJ-115 branch sets the buffer size according to the max_allowed_packet variable. This may be of some help for CONJ-116, too.

"Load data" packet size can be arbitrary small, like 1K ,which is guaranteed to work with any max_allowed_packet setting. The EOF is detected on the server side by the last packet.

I see that https://code.launchpad.net/~massimo-siani/mariadb-java-client/CONJ-115 has been updated since I last checked. I like the overall idea of the new solution.

The solution asks the server for the max_allowed_packet during connect. This will cause some overhead making connect a bit slower. For me this is not a problem since I use a connection pool to manage the connections, but it is a thing to consider.

The solution uses global state: the variable MySQLProtocol.maxAllowedPacket is a static variable that only has one value per JVM. This will make the solution not work when connecting to two different database servers that has different max_allowed_packet. I would recommend, as a general programming practice never to use global changeable state. To solve this specific problem maybe the size can be injected as the PacketOutputStream is created in MySQLProtocol.java?

The solution does not work if I set a higher max_allowed_packet than PacketOutputStream.MAX_PACKET_LENGTH. If I set

set GLOBAL max_allowed_packet = 128*1024*1024;

I will get a

java.sql.SQLNonTransientConnectionException: Got packets out of order

	at org.mariadb.jdbc.internal.SQLExceptionMapper.get(SQLExceptionMapper.java:136)

	at org.mariadb.jdbc.internal.SQLExceptionMapper.throwException(SQLExceptionMapper.java:106)

	at org.mariadb.jdbc.MySQLStatement.executeQueryEpilog(MySQLStatement.java:264)

	at org.mariadb.jdbc.MySQLStatement.execute(MySQLStatement.java:291)

	at org.mariadb.jdbc.MySQLStatement.execute(MySQLStatement.java:341)

	at org.mariadb.jdbc.GiganticLoadDataInfileTest.giganticLoadDataInfileTest(GiganticLoadDataInfileTest.java:52)

	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)

	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

	at java.lang.reflect.Method.invoke(Method.java:601)

	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)

	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)

	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)

	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)

	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)

	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)

	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:44)

	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:180)

	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:41)

	at org.junit.runners.ParentRunner$1.evaluate(ParentRunner.java:173)

	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)

	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)

	at org.junit.runners.ParentRunner.run(ParentRunner.java:220)

	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)

	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)

	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)

	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)

	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)

	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

Caused by: org.mariadb.jdbc.internal.common.QueryException: Got packets out of order

	at org.mariadb.jdbc.internal.mysql.MySQLProtocol.getResult(MySQLProtocol.java:947)

	at org.mariadb.jdbc.internal.mysql.MySQLProtocol.executeQuery(MySQLProtocol.java:996)

	at org.mariadb.jdbc.MySQLStatement.execute(MySQLStatement.java:284)

	... 26 more

My suggested fix for this is to use a Math.min(MAX_PACKET_LENGTH, MySQLProtocol.maxAllowedPacket)

Thanks for the feedback.