CASSGO-27 fix gocql-RetryPolicy-still-dont-use-query-idempotence

[OleksiienkoMykyta]

Closes #1803
RetryPolicy doesn't check the query's idempotency, but according to the specification queries with false idempotence shouldn't be retried.

[SiyaoIsHiding]

Hi, I think this won't fix the issue because of this line

cassandra-gocql-driver/query_executor.go

Lines 67 to 69 in 92b4056

	if !qry.IsIdempotent() || sp.Attempts() == 0 {
	return q.do(qry.Context(), qry, hostIter), nil
	}

the function do is only called when it's marked as NOT idempotent.

[joao-r-reis]

@SiyaoIsHiding that line you linked is to avoid speculative executions if the query isn't idempotent which seems correct to me... It's not really related to retries. If it's idempotent then run will be called which will then call do

[joao-r-reis]

@OleksiienkoMykyta can you add a test that reproduces the bug before this change to ensure we don't get a regression in the future? It should be possible to reproduce this by using a very small timeout

[OleksiienkoMykyta]

@OleksiienkoMykyta can you add a test that reproduces the bug before this change to ensure we don't get a regression in the future? It should be possible to reproduce this by using a very small timeout.

Yes, sure, I'll add some tests to cover this behavior.

[OleksiienkoMykyta]

@joao-r-reis I have added some tests, it would be nice if you would check it.
Also, I tried to test it with a timeout, but it doesn't reconcile query execution, just a single try, so I used a custom RetryPolicy.

[joao-r-reis]

TestQueryMultinodeWithMetrics is panicking because it assumes the retry policy runs for all hosts but now it doesn't because of the idempotency change.

[joao-r-reis]

Changes look good 👍 , still need to fix failing tests though.

[OleksiienkoMykyta]

Changes look good 👍 , still need to fix failing tests though.

I have fixed it. The issue was in test which should make retries but doesn't set idempotence to query explicitly, now everything works properly.

[joao-r-reis]

Please format your commit message following the rules stated in CONTRIBUTING.md and add this JIRA to the CHANGELOG.md in the Fixed section of Unreleased, thanks!

We need another committer +1 to merge this.

[jfleming-ic]

Would be nice to have a short summary of what was fixed here, so users don't need to lookup what this ticket is, convention seems to be
"message" (ticket)

So for this, something like this would suffice

"Retry policy now takes into account query idempotency (CASSGO-27)"

[Jacko161]

Sorry was on the wrong github account on my last comment - happy to tick this, but would be good to update the changelog message if possible.