Skip to content

Commit

Permalink
CASSGO-27 gocql RetryPolicy dont use query idempotence
Browse files Browse the repository at this point in the history
RetryPolicy doesn't check the query's idempotency,
but according to the specification queries with false idempotence shouldn't be retried.

patch by Mykyta Oleksiienko; reviewed by João Reis for CASSGO-27
  • Loading branch information
OleksiienkoMykyta committed Nov 13, 2024
1 parent 7b7e6af commit a262096
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- CASSGO-27

## [1.7.0] - 2024-09-23

This release is the first after the donation of gocql to the Apache Software Foundation (ASF)
Expand Down
50 changes: 50 additions & 0 deletions cassandra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"context"
"errors"
"fmt"
"github.com/stretchr/testify/require"
"io"
"math"
"math/big"
Expand Down Expand Up @@ -1066,6 +1067,55 @@ func matchSliceMap(t *testing.T, sliceMap []map[string]interface{}, testMap map[
}
}

type MyRetryPolicy struct {
}

func (*MyRetryPolicy) Attempt(q RetryableQuery) bool {
if q.Attempts() > 5 {
return false
}
return true
}

func (*MyRetryPolicy) GetRetryType(error) RetryType {
return Retry
}

func Test_RetryPolicyIdempotence(t *testing.T) {
session := createSession(t)
defer session.Close()

testCases := []struct {
name string
idempotency bool
expectedNumberOfTries int
}{
{
name: "with retry",
idempotency: true,
expectedNumberOfTries: 6,
},
{
name: "without retry",
idempotency: false,
expectedNumberOfTries: 1,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
q := session.Query("INSERT INTO gocql_test.not_existing_table(event_id, time, args) VALUES (?,?,?)", 4, UUIDFromTime(time.Now()), "test")

q.Idempotent(tc.idempotency)
q.RetryPolicy(&MyRetryPolicy{})
q.Consistency(All)

_ = q.Exec()
require.Equal(t, tc.expectedNumberOfTries, q.Attempts())
})
}
}

func TestSmallInt(t *testing.T) {
session := createSession(t)
defer session.Close()
Expand Down
2 changes: 1 addition & 1 deletion conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ func TestQueryMultinodeWithMetrics(t *testing.T) {
// 1 retry per host
rt := &SimpleRetryPolicy{NumRetries: 3}
observer := &testQueryObserver{metrics: make(map[string]*hostMetrics), verbose: false, logger: log}
qry := db.Query("kill").RetryPolicy(rt).Observer(observer)
qry := db.Query("kill").RetryPolicy(rt).Observer(observer).Idempotent(true)
if err := qry.Exec(); err == nil {
t.Fatalf("expected error")
}
Expand Down
2 changes: 1 addition & 1 deletion query_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (q *queryExecutor) do(ctx context.Context, qry ExecutableQuery, hostIter Ne

// Exit if the query was successful
// or no retry policy defined or retry attempts were reached
if iter.err == nil || rt == nil || !rt.Attempt(qry) {
if iter.err == nil || !qry.IsIdempotent() || rt == nil || !rt.Attempt(qry) {
return iter
}
lastErr = iter.err
Expand Down

0 comments on commit a262096

Please sign in to comment.