Skip to content

Commit

Permalink
Change RetryPolicy so it checks 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 and Jackson Fleming for CASSGO-27
  • Loading branch information
OleksiienkoMykyta authored and joao-r-reis committed Nov 19, 2024
1 parent adda8ea commit 109a892
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 @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Retry policy now takes into account query idempotency (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 109a892

Please sign in to comment.