Use `prepare(String)` for Prepared Statement preparation to prevent bind values from being cached

[sbied]

When updating to spring.data.cassandra 3.3.1 from an older version (that one "bundled" with spring-boot 2.3.8) I realized a poor write performance due to the fact that each write eats about 7k memory which then later results in painful full-gc's. After a full-gc, memory is reclaimed and the same procedure begins...

So, this being a typical cache-going-postal pattern, I searched for found the cache in Cassandra java driver class CQLPrepareAsyncProcessor responsible for this behavior. This cache holds PrepareRequests as key and CompletableFuture as values.

Since the write paths of spring.data.cassandra (*CassandraRepository, *CassandraTemplate) call session.prepare with fully filled-out SimpleStatements (not as one might expect with punched-out holes for the actual values to be filled in later during the binding process), a correct prepared statement is delivered, but each individual SimpleStatement (via DefaultPrepareRequest) is stored in the cache, yielding the "memory leak". The cache works with weak references, so a full-gc can reclaim memory.

For a small fix, it should be possible override the createPreparedStatement-Methods in the PreparedStatementHandler classes (themselves inner classes of *CassandraTemplate). Perhaps a SimpleStatementWrapper can do the job.

As a WORKAROUND I found out, that setting usePreparedStatement=false in *CassandraTemplate circumvents the problem (but leads to a performance loss of about 20% in my use case).

I hope, you can help here!

Best regards,

Sven

[mp911de]

call session.prepare with fully filled-out SimpleStatements

Do you have an actual example or reproducer? Spring Data Cassandra 3.3.x uses positional bind markers to prepare and run statements. We had once a bug #1172 that created issues for statements using IN keywords.

[sbied]

I think, use can produce the issue by writing any entity with

(Reactive)CassandraTemplate::insert, ::update

or using a *Repository on the entity with ::save oder ::insert.

The positional bind markers you mentioned, are still contained in the prepare request (a DefaultPrepareRequest will be created with the DefaultSimpleStatement inside it, which contains the markers). So, the preparing will see different PrepareRequests (see hashCode-methods), where you probably intended having the same PrepareRequest.

I think, by wrapping the SimpleStatement and leaving out the markers for the PrepareRequest, the problem could be fixed.
{of course not the most beautiful solution}.

[sbied]

PrepareRequest is inside the Cassandra-Driver, so the wrapping (punching-out holes) must occur before calling session.prepareAsync.

[mp911de]

With the investigation from your side, would you mind creating a pull request where we can explore the possibilities?

[sbied]

My environment is locked in in a vm with no access to the outside. But perhaps we can have a call, both sitting in Germany?

[samueldlightfoot]

One way to get the behaviour we expect is to ensure equals/hashCode for PrepareRequest only includes properties that will actually be used as part of the PreparedStatement. This would include dropping positional values and keeping consistency level, for example.

DefaultPrepareRequest does contain a String accepting constructor, of which we could pass in the parameterized (with bind markers) query String, however certain execution details of the query would be dropped that are usually contained on the SimpleStatement (consistency level etc).

The DataStax docs do say if you want to customize the current behaviour then you can implement your own PrepareRequest, so maybe this is the way forward However, I would be very keen to hear their thoughts on the current behaviour.

I think it should be also noted that due to the cache misses for each request, there will be an unnecessary amount of prepare requests being sent to the Cassandra nodes, which is waste of resources.

[mp911de]

Paging @adutra

[sbied]

As another solution, I think it might be worth considering to create the PreparedStatement once and store it with the associated CassandraPersistentEntity or a similar place. This might cause problems with consistency level and other options, of course.

[Rednas92-spec]

Why not remove the values before preparing?

    public PreparedStatement createPreparedStatement(CqlSession session) throws DriverException { 
        return session.prepare(simpleStatement.setPositionalValues(Collections.emptyList()));
    }

at org.springframework.data.cassandra.core.CassandraTemplate.PreparedStatementHandler#createPreparedStatement

In the javadocs of com.datastax.oss.driver.api.core.cql.SimpleStatement#setPositionalValues

The driver's built-in implementation is immutable, and returns a new instance from this
method. However custom implementations may choose to be mutable and return the same instance.

I don't think this is the most elegant solution as it says that custom implementations may choose to be mutable.

Let me know if I can contribute.

[mp911de]

I'm not sure that we want to host a fix/workaround for a driver issue.