Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
CONJ-151 - performance degraded after upgrading from 1.1.7 to 1.1.8
  • Loading branch information
@rusher
rusher committed May 29, 2015
1 parent 4b460e6 commit a163bdb
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 80 deletions.
29 changes: 17 additions & 12 deletions src/main/java/org/mariadb/jdbc/internal/common/packet/commands/StreamedQueryPacket.java
View file
Expand Up @@ -49,6 +49,7 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS

package org.mariadb.jdbc.internal.common.packet.commands;

import org.mariadb.jdbc.internal.SQLExceptionMapper;
import org.mariadb.jdbc.internal.common.QueryException;
import org.mariadb.jdbc.internal.common.packet.CommandPacket;
import org.mariadb.jdbc.internal.common.packet.PacketOutputStream;
Expand All @@ -58,28 +59,32 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS
import java.io.OutputStream;


public class StreamedQueryPacket implements CommandPacket
{
public class StreamedQueryPacket implements CommandPacket {

private final Query query;
private final int maxAllowedPacket;

public StreamedQueryPacket(final Query query)
{
public StreamedQueryPacket(final Query query, int maxAllowedPacket) {
this.query = query;
this.maxAllowedPacket = maxAllowedPacket;
}

public int send(final OutputStream ostream) throws IOException,
QueryException
{
public int send(final OutputStream ostream) throws IOException, QueryException {
byte[] queryStream = query.sqlByteArray();
if (maxAllowedPacket > 0 && queryStream.length > maxAllowedPacket) {
throw new QueryException("Packet for query is too large ("
+ queryStream.length
+ " > "
+ maxAllowedPacket
+ "). You can change this value on the server by setting the max_allowed_packet' variable.",
-1, SQLExceptionMapper.SQLStates.UNDEFINED_SQLSTATE.getSqlState());
}
PacketOutputStream pos = (PacketOutputStream)ostream;
pos.startPacket(0);
pos.write(0x03);
query.writeTo(pos);
ostream.write(queryStream, 0, queryStream.length);
pos.finishPacket();
return 0;
}

public int getPacketLength() {
return query.getPacketLength();
}

}
44 changes: 8 additions & 36 deletions src/main/java/org/mariadb/jdbc/internal/common/query/MySQLParameterizedQuery.java
View file
Expand Up @@ -50,12 +50,9 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS

import org.mariadb.jdbc.internal.common.QueryException;
import org.mariadb.jdbc.internal.common.query.parameters.ParameterHolder;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.nio.charset.Charset;
import java.util.List;

import static org.mariadb.jdbc.internal.common.Utils.createQueryParts;
Expand Down Expand Up @@ -121,17 +118,18 @@ public void validate() throws QueryException{
}


public void writeTo(final OutputStream os) throws IOException, QueryException {

public byte[] sqlByteArray() throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
if(queryPartsArray.length == 0) {
throw new AssertionError("Invalid query, queryParts was empty");
}
os.write(queryPartsArray[0]);
baos.write(queryPartsArray[0]);
for(int i = 1; i<queryPartsArray.length; i++) {
parameters[i-1].writeTo(os);
parameters[i-1].writeTo(baos);
if(queryPartsArray[i].length != 0)
os.write(queryPartsArray[i]);
baos.write(queryPartsArray[i]);
}
return baos.toByteArray();
}


Expand Down Expand Up @@ -186,38 +184,12 @@ public String toString() {
*/
public String toSQL() {
try {
final ByteArrayOutputStream os = new ByteArrayOutputStream();
writeTo(os);
String sql = new String(os.toByteArray(), Charset.forName("UTF8"));
return sql;
} catch (QueryException qe) {
return "";
return new String(sqlByteArray());
} catch (IOException e) {
return "";
}
}

private String toSQL2() throws UnsupportedEncodingException {
if(queryPartsArray.length == 0) {
return "";
}
String result;
result = new String(queryPartsArray[0], "UTF-8");
for(int i = 1; i<queryPartsArray.length; i++) {
result += parameters[i-1];
if(queryPartsArray[i].length != 0)
result += new String(queryPartsArray[i], "UTF-8");
}
return result;
}

@Override
public int getPacketLength() {
try {
return toSQL2().getBytes("UTF-8").length;
} catch (UnsupportedEncodingException e) {
return -1;
}
}


}
12 changes: 2 additions & 10 deletions src/main/java/org/mariadb/jdbc/internal/common/query/MySQLQuery.java
View file
Expand Up @@ -49,9 +49,6 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS

package org.mariadb.jdbc.internal.common.query;
import org.mariadb.jdbc.internal.common.QueryException;



import java.io.IOException;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
Expand Down Expand Up @@ -85,8 +82,8 @@ public int length() {
return queryToSend.length;
}

public void writeTo(final OutputStream os) throws IOException {
os.write(queryToSend, 0, queryToSend.length);
public byte[] sqlByteArray() {
return queryToSend;
}

public String getQuery() {
Expand Down Expand Up @@ -115,10 +112,5 @@ public String toString() {
return query;
}

@Override
public int getPacketLength() {
return length();
}


}
6 changes: 1 addition & 5 deletions src/main/java/org/mariadb/jdbc/internal/common/query/Query.java
View file
Expand Up @@ -50,15 +50,11 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS
package org.mariadb.jdbc.internal.common.query;

import org.mariadb.jdbc.internal.common.QueryException;

import java.io.IOException;
import java.io.OutputStream;


public interface Query {
void writeTo(OutputStream os) throws IOException, QueryException;
byte[] sqlByteArray() throws IOException;
String getQuery();
QueryType getQueryType();
void validate() throws QueryException;
int getPacketLength();
}
14 changes: 2 additions & 12 deletions src/main/java/org/mariadb/jdbc/internal/mysql/MySQLProtocol.java
View file
Expand Up @@ -1016,18 +1016,8 @@ public QueryResult executeQuery(final Query dQuery, boolean streaming) throws Qu
dQuery.validate();
log.log(Level.FINEST, "Executing streamed query: {0}", dQuery);
this.moreResults = false;
final StreamedQueryPacket packet = new StreamedQueryPacket(dQuery);

int packetLength = packet.getPacketLength();
if (this.maxAllowedPacket > 0 && packetLength > 0 && packetLength > this.maxAllowedPacket) {
throw new QueryException("Packet for query is too large ("
+ packetLength
+ " > "
+ this.maxAllowedPacket
+ "). You can change this value on the server by setting the max_allowed_packet' variable.",
-1, SQLExceptionMapper.SQLStates.UNDEFINED_SQLSTATE.getSqlState());
}

final StreamedQueryPacket packet = new StreamedQueryPacket(dQuery, this.maxAllowedPacket);

try {
packet.send(writer);
} catch (IOException e) {
Expand Down
26 changes: 21 additions & 5 deletions src/test/java/org/mariadb/jdbc/ConnectionTest.java
View file
Expand Up @@ -2,13 +2,12 @@

import static org.junit.Assert.*;

import java.io.InputStream;
import java.io.OutputStream;
import java.lang.reflect.Field;
import java.sql.Connection;
import java.sql.*;
import java.io.UnsupportedEncodingException;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.SQLPermission;
import java.sql.Statement;
import java.util.Arrays;
import java.util.Properties;
import java.util.concurrent.Executor;

Expand Down Expand Up @@ -176,11 +175,28 @@ public void maxAllowedPackedExceptionIsPrettyTest() throws SQLException, Unsuppo
assertTrue(e.getMessage().contains("max_allowed_packet"));
} catch (Exception e) {
fail("The previous statement should throw an SQLException not a general Exception");
}

//added in CONJ-151 to check the 2 differents type of query
PreparedStatement preparedStatement = connection.prepareStatement("INSERT INTO dummy VALUES (?)");
byte [] arr = new byte[maxAllowedPacket + 1000];
Arrays.fill(arr, (byte) 'a');
preparedStatement.setBytes(1,arr);
preparedStatement.addBatch();
try {
preparedStatement.executeBatch();
fail("The previous statement should throw an SQLException");
} catch (SQLException e) {
assertTrue(e.getMessage().contains("max_allowed_packet"));
} catch (Exception e) {
fail("The previous statement should throw an SQLException not a general Exception");
} finally {
statement.execute("DROP TABLE dummy");
}

}


@Test
public void isValid_testWorkingConnection() throws SQLException {
assertTrue(connection.isValid(0));
Expand Down

4 comments on commit a163bdb

@vaintroub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no need in sqlToByteArray() eithe (apart from possibly debugging), and no need in toSQL()..
If you have prepared statement with multimegabyte input streams, materializing them in memory is probably not a good idea. Also, converting to byte array adds an unneeded copy of the data, which affects performance.

@vaintroub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If is a need to check for packet length, to "beautify" the exception, and avoid server closing the socket, this is best done when the actual packet is sent. This is in

https://github.com/MariaDB/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/internal/common/packet/PacketOutputStream.java

in internalFlush(), you can check for datalen > max packet length. Before you throw an exception, you'd need to reset the PacketOutputStream's buffer (position to 0) and seqNo to 0, so that stream remains usable after the exception, i you do not want this exception to be fatal.
if you want this exception to be fatal, you'd probably need to close the connection

@rusher
Copy link
Collaborator Author

@rusher rusher commented on a163bdb May 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, truly, your solution is more effective (code length and perf), I'll change that.

@vaintroub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Please sign in to comment.