0

I am caching prepared statement so that I don't have to prepare it again while working with datastax java driver (cassandra).. Below is my code and it works:

  private static final ConcurrentHashMap<String, PreparedStatement> cache = new ConcurrentHashMap<>();

  public ResultSetFuture send(final String cql, final Object... values) {
    return executeWithSession(new SessionCallable<ResultSetFuture>() {
      @Override
      public ResultSetFuture executeWithSession(Session session) {
        BoundStatement bs = getStatement(cql, values);
        bs.setConsistencyLevel(consistencyLevel);
        return session.executeAsync(bs);
      }
    });
  }

  private BoundStatement getStatement(final String cql, final Object... values) {
    Session session = getSession();
    PreparedStatement ps = cache.get(cql);
    // no statement cached, create one and cache it now.
    // below line is causing thread safety issue..
    if (ps == null) {
      ps = session.prepare(cql);
      PreparedStatement old = cache.putIfAbsent(cql, ps);
      if (old != null)
        ps = old;
    }
    return ps.bind(values);
  }

But problem is send method will be called by multiple threads so I suspect my getStatement method is not thread safe because of if (ps == null) check.. How can I make it thread safe?

I wanted to avoid using synchronize keyword so wanted to see if there is any better way. I am working with Java 7 as of now.

flash
  • 1,545
  • 9
  • 45
  • 107
  • Err, synchronization? – user207421 Feb 20 '20 at 23:11
  • I wanted to avoid using `synchronize` keyword. Is there any better way? – flash Feb 20 '20 at 23:12
  • A `PreparedStatement` belongs to a `Connection`, and while `Connection` objects are *thread-safe*, they are ***not concurrent***, so you're losing more with the implied synchronization of the `Connection` than you're gaining by caching the `PreparedStatement`. It was a good idea, but you should reject it immediately. --- [Is java.sql.Connection thread safe?](https://stackoverflow.com/q/1531073/5221149) – Andreas Feb 21 '20 at 00:01
  • If you run the statement a lot, you could cache it for the thread in a `ThreadLocal`, but some JDBC drivers will cache prepared statements for you anyway, so you might just be complicating your code for nothing. – Andreas Feb 21 '20 at 00:02
  • *FYI:* [`java.sql.PreparedStatement`](https://docs.oracle.com/en/java/javase/13/docs/api/java.sql/java/sql/PreparedStatement.html) doesn't have a `bind()` method. – Andreas Feb 21 '20 at 00:06
  • @Andreas This is datastax java driver code. I am working with cassandra so it’s not jdbc. – flash Feb 21 '20 at 00:07

1 Answers1

1

You can use computeIfAbsent instead. Per documentation it's:

If the specified key is not already associated with a value, attempts to compute its value using the given mapping function and enters it into this map unless null. The entire method invocation is performed atomically, so the function is applied at most once per key. Some attempted update operations on this map by other threads may be blocked while computation is in progress, so the computation should be short and simple, and must not attempt to update any other mappings of this map.

The code would look as following:

private BoundStatement getStatement(final String cql, final Object... values) {
   PreparedStatement pr = cache.computeIfAbsent(
        query,  key -> session.prepare(key));
   return pr.bind(values);
}

although, because preparation could take longer time, it's not recommended... I would instead use explicit locks...

P.S. Please note that Java driver 4.x has built-in cache, so it's ok to prepare same statement multiple times.

Alex Ott
  • 64,084
  • 6
  • 72
  • 107
  • thanks for your suggestion. I am still working with Java 7. How can we write this with Java 7? – flash Feb 21 '20 at 17:03
  • Hmmm, I don’t see putIfAbsent in Java 7 API... but yes, you can use explicit locks for that – Alex Ott Feb 21 '20 at 17:07
  • can u provide an example for that? or you mean just synchronizing just above `if` check with `synchronize` keyword? – flash Feb 21 '20 at 17:12