ExecuteBatchStreamingTest fails sporadically for the C* 4.0

[ivansenic]

Unfortunately, the gRPC test ExecuteBatchStreamingTest.manyStreamingBatch fails sporadically even after merging the #1661. Seems that two retries are not enough and that we can hit the situation that both retries also fail.
I don't see any other solution, than to increase the number of retries. We might leave it being limited, but maybe 2 is too less.

[ivansenic]

@mpenick what do you think?

[tatu-at-datastax]

Is there any delay/wait between retries? 2 may be too low but as importantly retries usually should not be back-to-back. Especially since in this case failure is immediate I think (no external calls, access to cache is very fast).

[ivansenic]

There is no delay as there was no option to add this in the existing grpc retry policy and the implementation around it..

[tatu-at-datastax]

@ivansenic that makes sense, was guessing that would be the case. Not sure how but it seems important here that there's no busy loop attempt because that may take a very high repeat count without guarantees.

I guess we could start with higher retry count, still, but without some sort of "Thread.yield()" equivalent (that is, something similar that would work in this context) not sure how much simple increase of back-to-back retries would help.
Although maybe gRPC is smart enough to do something like this by default (and not just assume there must be slower external call or I/O)

[ivansenic]

@tatu-at-datastax I think since we are using the direct executor in gRPC, there must not be any thead blocks.. So no this is not an option..

[mpenick]

I guess I need to understand why it's being invalidated so much that it's returning unprepared so often. Is that something that can be fixed?

[mpenick]

I see: https://github.com/apache/cassandra/blob/cassandra-4.0.3/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L580-L583

So if the same statement is prepared at the same time either one of those checks could fail because of a simultaneous eviction. This is not ideal, but I think the way to fix this is to synchronize prepares, at least in gRPC, so that it reduces the chance of eviction. However, this is likely to affect all API that eagerly prepare, so maybe all prepare could be synchronized for the C* 4.X persistence?

[tatu-at-datastax]

@ivansenic yes, understood, was expecting that sleep/yield won't work. But effectively we need something like that (but not those :) ).

[tatu-at-datastax]

@mpenick Wow. That cache handling logic is seriously convoluted, with fixes wrt keyspace/no-keyspace cases and all. It is difficult to trust that to work for all cases...

But if that is to be retained (which for performance I assume needs to), instead of global sync lock it is probably possible to split lock into N different ones on module of hash code.

[mpenick]

But if that is to be retained (which for performance I assume needs to), instead of global sync lock it is probably possible to split lock into N different ones on module of hash code.

Sounds good.