Skip to content

Commit a262096

Browse files
CASSGO-27 gocql RetryPolicy dont use query idempotence
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
1 parent 7b7e6af commit a262096

File tree

4 files changed

+54
-2
lines changed

4 files changed

+54
-2
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414

1515
### Fixed
1616

17+
- CASSGO-27
18+
1719
## [1.7.0] - 2024-09-23
1820

1921
This release is the first after the donation of gocql to the Apache Software Foundation (ASF)

cassandra_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"context"
3333
"errors"
3434
"fmt"
35+
"github.com/stretchr/testify/require"
3536
"io"
3637
"math"
3738
"math/big"
@@ -1066,6 +1067,55 @@ func matchSliceMap(t *testing.T, sliceMap []map[string]interface{}, testMap map[
10661067
}
10671068
}
10681069

1070+
type MyRetryPolicy struct {
1071+
}
1072+
1073+
func (*MyRetryPolicy) Attempt(q RetryableQuery) bool {
1074+
if q.Attempts() > 5 {
1075+
return false
1076+
}
1077+
return true
1078+
}
1079+
1080+
func (*MyRetryPolicy) GetRetryType(error) RetryType {
1081+
return Retry
1082+
}
1083+
1084+
func Test_RetryPolicyIdempotence(t *testing.T) {
1085+
session := createSession(t)
1086+
defer session.Close()
1087+
1088+
testCases := []struct {
1089+
name string
1090+
idempotency bool
1091+
expectedNumberOfTries int
1092+
}{
1093+
{
1094+
name: "with retry",
1095+
idempotency: true,
1096+
expectedNumberOfTries: 6,
1097+
},
1098+
{
1099+
name: "without retry",
1100+
idempotency: false,
1101+
expectedNumberOfTries: 1,
1102+
},
1103+
}
1104+
1105+
for _, tc := range testCases {
1106+
t.Run(tc.name, func(t *testing.T) {
1107+
q := session.Query("INSERT INTO gocql_test.not_existing_table(event_id, time, args) VALUES (?,?,?)", 4, UUIDFromTime(time.Now()), "test")
1108+
1109+
q.Idempotent(tc.idempotency)
1110+
q.RetryPolicy(&MyRetryPolicy{})
1111+
q.Consistency(All)
1112+
1113+
_ = q.Exec()
1114+
require.Equal(t, tc.expectedNumberOfTries, q.Attempts())
1115+
})
1116+
}
1117+
}
1118+
10691119
func TestSmallInt(t *testing.T) {
10701120
session := createSession(t)
10711121
defer session.Close()

conn_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ func TestQueryMultinodeWithMetrics(t *testing.T) {
435435
// 1 retry per host
436436
rt := &SimpleRetryPolicy{NumRetries: 3}
437437
observer := &testQueryObserver{metrics: make(map[string]*hostMetrics), verbose: false, logger: log}
438-
qry := db.Query("kill").RetryPolicy(rt).Observer(observer)
438+
qry := db.Query("kill").RetryPolicy(rt).Observer(observer).Idempotent(true)
439439
if err := qry.Exec(); err == nil {
440440
t.Fatalf("expected error")
441441
}

query_executor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (q *queryExecutor) do(ctx context.Context, qry ExecutableQuery, hostIter Ne
166166

167167
// Exit if the query was successful
168168
// or no retry policy defined or retry attempts were reached
169-
if iter.err == nil || rt == nil || !rt.Attempt(qry) {
169+
if iter.err == nil || !qry.IsIdempotent() || rt == nil || !rt.Attempt(qry) {
170170
return iter
171171
}
172172
lastErr = iter.err

0 commit comments

Comments
 (0)