[
https://issues.apache.org/jira/browse/DERBY-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13571307#comment-13571307
]
Knut Anders Hatlen commented on DERBY-6053:
-------------------------------------------
Some post-commit comments:
- As far as I can see, all accesses to isolationLevelPreparedStmts are
synchronized on the Connection instance, so maybe it could be an unsynchronized
HashMap instead of a synchronized Hashtable?
- isolationLevelPreparedStmts is initialized to a non-null value and never
changed, so checking that it's not null before accessing it shouldn't be
necessary. (Maybe also declare the field as final to make it explicit that it
is never changed from the initial non-null value.)
- I think the declaration of isolationLevelPreparedStmts should include type
parameters, so that it's easier to access the values stored in it:
HashMap<String,PreparedStatement> isolationLevelPreparedStmts = new
HashMap<String,PreparedStatement>();
- There's a compiler warning when I build the code. This line looks wrong, and
I suppose that's what the compiler is warning us about:
for (Iterator<PreparedStatement> it =
isolationLevelPreparedStmts.keySet().iterator();
The keys in the hash table are strings, not prepared statements. So it should
say Iterator<String>. (If the type parameters had been included in the
declaration of the field, the compiler would have known it was wrong, and it
would have produced an error instead of a warning.)
However, it's much simpler to use a for-each loop, so I suggest that the loop
is changed to something like this:
for (PreparedStatement ps : isolationLevelPreparedStmts.values()) {
try {
ps.close();
} catch (SQLException se) {
// ...
}
}
- When the declaration of isolationLevelPreparedStmts is changed to include the
type parameters <String,PreparedStatement>, it is no longer necessary to cast
the value returned from isolationLevelPreparedStmts.get(levelString), as the
compiler already knows it returns a PreparedStatement.
> Client should use a prepared statement rather than regular statement for
> Connection.setTransactionIsolation
> -----------------------------------------------------------------------------------------------------------
>
> Key: DERBY-6053
> URL: https://issues.apache.org/jira/browse/DERBY-6053
> Project: Derby
> Issue Type: Improvement
> Components: Network Client
> Reporter: Kathey Marsden
> Assignee: Mamta A. Satoor
> Fix For: 10.10.0.0
>
> Attachments: DERBY6053_patch1_diff.txt
>
>
> o.a.d.client.am.Connection setTransactionIsolation() uses a Statement which
> it builds up each time for setTransactionIsolation() is called.
> private Statement setTransactionIsolationStmt = null;
> ...
> setTransactionIsolationStmt =
> createStatementX(java.sql.ResultSet.TYPE_FORWARD_ONLY,
> java.sql.ResultSet.CONCUR_READ_ONLY,
> holdability());
> ....
> private void setTransactionIsolationX(int level)
> ...
> setTransactionIsolationStmt.executeUpdate(
> "SET CURRENT ISOLATION = " + levelString);
> It would be better for performance and also for avoid possible garbage
> collection issues, to have a single prepared statement with a parameter
> marker.
> The program below shows repeated calls to setTransactionIsolation.
> import java.sql.*;
> import java.net.*;
> import java.io.*;
> import org.apache.derby.drda.NetworkServerControl;
> /**
> * Client template starts its own NetworkServer and runs some SQL against it.
> * The SQL or JDBC API calls can be modified to reproduce issues
> *
> */public class SetTransactionIsolation {
> public static Statement s;
>
> public static void main(String[] args) throws Exception {
> try {
> // Load the driver. Not needed for network server.
>
> Class.forName("org.apache.derby.jdbc.ClientDriver");
> // Start Network Server
> startNetworkServer();
> // If connecting to a customer database. Change the URL
> Connection conn = DriverManager
>
> .getConnection("jdbc:derby://localhost:1527/wombat;create=true");
> // clean up from a previous run
> s = conn.createStatement();
> try {
> s.executeUpdate("DROP TABLE T");
> } catch (SQLException se) {
> if (!se.getSQLState().equals("42Y55"))
> throw se;
> }
> for (int i = 0; i < 50000; i++) {
>
> conn.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ);
>
> conn.setTransactionIsolation(Connection.TRANSACTION_SERIALIZABLE);
> }
>
> // rs.close();
> // ps.close();
> runtimeInfo();
> conn.close();
> // Shutdown the server
> shutdownServer();
> } catch (SQLException se) {
> while (se != null) {
> System.out.println("SQLState=" + se.getSQLState()
> + se.getMessage());
> se.printStackTrace();
> se = se.getNextException();
> }
> }
> }
>
> /**
> * starts the Network server
> *
> */
> public static void startNetworkServer() throws SQLException {
> Exception failException = null;
> try {
>
> NetworkServerControl networkServer = new NetworkServerControl(
> InetAddress.getByName("localhost"), 1527);
>
> networkServer.start(new PrintWriter(System.out));
>
> // Wait for the network server to start
> boolean started = false;
> int retries = 10; // Max retries = max seconds to wait
>
> while (!started && retries > 0) {
> try {
> // Sleep 1 second and then ping the network server
> Thread.sleep(1000);
> networkServer.ping();
>
> // If ping does not throw an exception the server has
> // started
> started = true;
> } catch (Exception e) {
> retries--;
> failException = e;
> }
>
> }
>
> // Check if we got a reply on ping
> if (!started) {
> throw failException;
> }
> } catch (Exception e) {
> SQLException se = new SQLException("Error starting network
> server");
> se.initCause(failException);
> throw se;
> }
> }
>
> public static void shutdownServer() throws Exception {
> NetworkServerControl networkServer = new NetworkServerControl(
> InetAddress.getByName("localhost"), 1527);
> networkServer.shutdown();
> }
>
> public static void runtimeInfo() throws Exception {
> NetworkServerControl networkServer = new NetworkServerControl(
> InetAddress.getByName("localhost"), 1527);
> System.out.println(networkServer.getRuntimeInfo());
> }
>
> }
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
