Skip to content

Commit 109a892

Browse files
OleksiienkoMykytajoao-r-reis
authored andcommitted
Change RetryPolicy so it checks 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 and Jackson Fleming for CASSGO-27
1 parent adda8ea commit 109a892

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
@@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616

1717
### Fixed
1818

19+
- Retry policy now takes into account query idempotency (CASSGO-27)
20+
1921
## [1.7.0] - 2024-09-23
2022

2123
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)