Thread Safety issues in the multithreading code

Question

I am working on a project in which I have two tables in a different database with different schemas. So that means I have two different connection parameters for those two tables to connect using JDBC-

Let's suppose below is the config.property file-

TABLES: table1 table2

#For Table1
table1.url: jdbc:mysql://localhost:3306/garden
table1.user: gardener
table1.password: shavel
table1.driver: jdbc-driver
table1.percentage: 80

#For Table2
table2.url: jdbc:mysql://otherhost:3306/forest
table2.user: forester
table2.password: axe
table2.driver: jdbc-driver
table2.percentage: 20

Below method will read the above config.properties file and make a ReadTableConnectionInfo object for each tables.

  private static void readPropertyFile() throws IOException {

    prop.load(Read.class.getClassLoader().getResourceAsStream("config.properties"));

    tableNames = Arrays.asList(prop.getProperty("TABLES").split(" "));

    for (String arg : tableNames) {

        ReadTableConnectionInfo ci = new ReadTableConnectionInfo();

        String url = prop.getProperty(arg + ".url");
        String user = prop.getProperty(arg + ".user");
        String password = prop.getProperty(arg + ".password");
        String driver = prop.getProperty(arg + ".driver");
        double percentage = Double.parseDouble(prop.getProperty(arg + ".percentage"));

       ci.setUrl(url);
       ci.setUser(user);
       ci.setPassword(password);
       ci.setDriver(driver);
       ci.setPercentage(percentage);

        tableList.put(arg, ci);

    }
}

Below is the ReadTableConnectionInfo class that will hold all the table connection info for a particular table.

public class ReadTableConnectionInfo {

    public String url;
    public String user;
    public String password;
    public String driver;
    public String percentage;

    public String getUrl() {
        return url;
    }

    public void setUrl(String url) {
        this.url = url;
    }

    public String getUser() {
        return user;
    }

    public void setUser(String user) {
        this.user = user;
    }

    public String getPassword() {
        return password;
    }

    public void setPassword(String password) {
        this.password = password;
    }

    public String getDriver() {
        return driver;
    }

    public void setDriver(String driver) {
        this.driver = driver;
    }
}

Now I am creating ExecutorService for specified number of threads and passing this tableList object (that I created by reading the config.property file) to constructor of ReadTask class -

// create thread pool with given size
ExecutorService service = Executors.newFixedThreadPool(10);

for (int i = 0; i < 10; i++) {
    service.submit(new ReadTask(tableList));
}

Below is my ReadTask class that implements Runnable interface in which each thread will make two connections for each table in the starting before doing anything meaningful.

class ReadTask implements Runnable {

    private Connection[] dbConnection = null;
    private ConcurrentHashMap<ReadTableConnectionInfo, Connection> tableStatement = new ConcurrentHashMap<ReadTableConnectionInfo, Connection>();
private PreparedStatement preparedStatement = null;
private ResultSet rs = null;

private static Random random = new SecureRandom();

    public ReadTask(LinkedHashMap<String, ReadTableConnectionInfo> tableList) {
        this.tableLists = tableList;
    }


    @Override
    public void run() {

        try {

            int j = 0;
            dbConnection = new Connection[tableList.size()];

            //loop around the map values and make the connection list
            for (ReadTableConnectionInfo ci : tableList.values()) {

                dbConnection[j] = getDBConnection(ci.getUrl(), ci.getUser(), ci.getPassword(), ci.getDriver());
                tableStatement.putIfAbsent(ci, dbConnection[j]);

                j++;
        }

long startTime = System.currentTimeMillis();
long endTime = startTime + (10 * 60 * 1000);

while (System.currentTimeMillis() <= endTime) {


    double randomNumber = random.nextDouble() * 100.0;
    ReadTableConnectionInfo table = selectRandomConnection(randomNumber);

    for (Map.Entry<ReadTableConnectionInfo, Connection> entry : tableStatement.entrySet()) {

        //The below if condition will be fine from ThreadSafety issues?
        if (entry.getKey().getTableName().equals(table.getTableName())) {

            final String id = generateRandomId(random);
            final String selectSql = generateRandomSQL(table);

            preparedStatement = entry.getValue().prepareCall(selectSql);
            preparedStatement.setString(1, id);

            rs = preparedStatement.executeQuery();
        }
            }
        }
        }
     }

private String generateRandomSQL(ReadTableConnectionInfo table) {

    int rNumber = random.nextInt(table.getColumns().size());

    List<String> shuffledColumns = new ArrayList<String>(table.getColumns());
    Collections.shuffle(shuffledColumns);

    String columnsList = "";

    for (int i = 0; i < rNumber; i++) {
        columnsList += ("," + shuffledColumns.get(i));
    }

    final String sql = "SELECT ID" + columnsList + "  from "
            + table.getTableName() + " where id = ?";

    return sql;
}


private ReadTableConnectionInfo selectRandomConnection(double randomNumber) {

    double limit = 0;
    for (ReadTableConnectionInfo ci : tableLists.values()) {
        limit += ci.getPercentage();
        if (random.nextDouble() < limit) {
            return ci;
        }
        throw new IllegalStateException();
    }
    return null;
}

 }

private Connection getDBConnection(String url, String username, String password, String driver) {

    Connection dbConnection = null;

    try {
        Class.forName(driver);

        dbConnection = DriverManager.getConnection(url, username, password);
    }

    return dbConnection;
}

In my above try block, I am making dbConnection array for storing two different database connections. And then I have a tableStatement as ConcurrentHashMap in which I am storing ReadTableConnectionInfo object with its dbConnection. For example Table1 object will have Table1 connection in the tableStatement ConcurrentHashMap.

And then after that I am applying this logic-

/* Generate random number and check to see whether that random number
 * falls between 1 and 80, if yes, then choose table1
 * and then use table1 connection and statement that I made above and do a SELECT * on that table.
 * If that random numbers falls between 81 and 100 then choose table2 
 * and then use table2 connection and statement and do a SELECT * on that table
 */

1) Generating Random number between 1 and 100.
2) If that random number is less than table1.getPercentage() then I am choosing `table1` and then use `table1 connection` object to make a SELECT sql call to that database.
else choose `table2` and then use `table2 connection object` to make a SELECT sql call to that database.

Problem Statement:-

Are there any potential thread safety issues here in my run method?

Answer 1

In summary, you have 2 shared resources: the tableList variable and the database.

Regarding tableList, if:

• tableList is fully populated before you submit the tasks to the executor service AND
• it is populated in the same thread as the main thread that creates the executor and submits the tasks AND
• it is not modified after that

THEN the access in read-only mode to that shared variable is thread-safe (thanks to the semantics of the submit method which creates a happens-before relationship.

Actions in a thread prior to the submission of a Runnable or Callable task to an ExecutorService happen-before any actions taken by that task.

Regarding the access to the database, DriverManager.getConnection() is thread safe and you don't share the connection object across threads so you are fine (although it would probably be more efficient to use a connection pool).

So your code looks thread safe to me.