0

I am relatively new to java and database and therefore asking your help for my code optimization. I have around 20 text files with comma separated values.Each text files has around 10000 lines Based on the the 3rd value in each line, I insert the data into different tables. Each time I check the 3rd value and use different methods to save this data. My code is as follows. Could someone please tell me if this is the proper way to do this operation. Thanks in advance.

public  void readSave() throws SQLException
{
    File dir = new File("C:\\Users\\log");
    String url = Config.DB_URL;
    String user= Config.DB_USERNAME;
    String password= Config.DB_PASSWORD;
    con= DriverManager.getConnection(url, user, password);
    con.setAutoCommit(false);
    String currentLine;
    if (!dir.isDirectory())
        throw new IllegalStateException();
    for (File file : dir.listFiles()) {
        BufferedReader br;
        try {
            br = new BufferedReader(new FileReader(file));
            while ((currentLine = br.readLine()) != null) {
                List<String> values = Arrays.asList(currentLine.split(","));
                if (values.get(2).contentEquals("0051")) 
                    save0051(values,con);
                else if(values.get(2).contentEquals("0049"))
                    save0049(values,con);
                else if(values.get(2).contentEquals("0021"))
                    save0021(values,con);
                else if(values.get(2).contentEquals("0089"))
                    save0089(values,con);
                if(statement!=null)
                    statement.executeBatch();
            }
        } catch (FileNotFoundException e1) {
            // TODO Auto-generated catch block
            e1.printStackTrace();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (SQLException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
    try { 
        con.commit();
        statement.close();
        con.close();
    } 
    catch (Exception e) {}
}

private void save0051(List<String> values, Connection connection) throws SQLException {
    // TODO Auto-generated method stub
    String WRITE_DATA = "INSERT INTO LOCATION_DATA"
            + "(loc_id, timestamp, message_id" +
            ) VALUES (?,?,?)";
    try {
        statement = connection.prepareStatement(WRITE_DATA);
        statement.setString(1, values.get(0));
        statement.setLong(2, Long.valueOf(values.get(1)));
        statement.setInt(3, Integer.valueOf(values.get(2)));
        statement.addBatch();
    } catch (SQLException e) {
        e.printStackTrace();
        System.out.println("Could not save to DB, error: " + e.getMessage());
    }
    return;
}
java_learner
  • 182
  • 3
  • 13
  • 1
    it seems your are preparing the statements on each function call, would be better if you prepared them once, with different statement object names and just accessed them in the varous functions – Akash May 02 '13 at 13:14

4 Answers4

5
  1. Don't create the database connection in the loop. This is an expensive operation and you should create it only once.
  2. Don't create the PreparedStatement in the loop. Create it once and reuse it.
  3. Don't commit after every single INSERT. Read about using batches for inserting. This reduces the "commit-overhead" dramatically if you only make a commit every let's say 200 INSERTs.
Kai
  • 38,985
  • 14
  • 88
  • 103
3

If this is going to be performance critical I'd suggest a few changes.

  1. Move the connection creation out of the loop, you don't want to be doing that thousands of times.
  2. Since each function is repeatedly making the same query, you can cache the PreparedStatements, and repeatedly execute them rather than recreating them with each query. This way the database will only need to optimize the query once, and each query will only transmit the data for the query as opposed to the entire query and the data.
Varun Madiath
  • 3,152
  • 4
  • 30
  • 46
  • No problem. I would however heed @user714965's advice of batching the inserts. It's the smart thing to do. In fact I'd go further to say that you should check whether you really need transactions here, and simply remove the transaction related code if you don't need it. Finally if you do need this and you're running it against a production database, it might be a good idea to rate limit your inserts so you don't throw off the DB with the high write workload. – Varun Madiath May 02 '13 at 13:31
  • Thanks for the update. As suggested, I moved the connection creation and – java_learner May 03 '13 at 06:16
  • do I need 4 different PrepareStatements as I am inserting into 4 different tables and pass this as an argument to each method? – java_learner May 03 '13 at 06:58
0

Just spotted that batch insert was already mentioned but here is a nice tutorial page I came across, I think he explains it quite well

howiewylie
  • 171
  • 4
0

Use JDBC Batch INSERT,executeBatch() is faster as insert is made in one shot as a list. see http://javarevisited.blogspot.com/2013/01/jdbc-batch-insert-and-update-example-java-prepared-statement.html Efficient way to do batch INSERTS with JDBC http://www.java2s.com/Code/Java/Database-SQL-JDBC/BatchUpdateInsert.htm

Community
  • 1
  • 1
abishkar bhattarai
  • 7,371
  • 8
  • 49
  • 66