Fix bug in query executor that prevented retries from ceasing (#1164)

If a retry policy is defined for the session/query, retries wouldn't
stop even if the retry policy returned false in the call to `Attempt()`.
The bug was introduced in #1151. As long as queries keep failing,
the query would be retried. The unit test that checks the retries
being executed now checks for an exact number of query retries, making
it a regression test as well.

conn_test.go

@@ -336,7 +336,7 @@ func TestQueryRetry(t *testing.T) {
 		}
 	}()
 
-	rt := &testRetryPolicy{numRetries: 10, t: t, attemptTimeout: time.Millisecond * 25}
+	rt := &testRetryPolicy{numRetries: 2, t: t, attemptTimeout: time.Millisecond * 25}
 	queryCtx, cancel := context.WithTimeout(context.Background(), time.Millisecond*90)
 	defer cancel()
 	qry := db.Query("slow").RetryPolicy(rt).Observer(&testQueryObserver{}).WithContext(queryCtx)
@@ -350,13 +350,10 @@ func TestQueryRetry(t *testing.T) {
 
 	numQueries := atomic.LoadUint64(&srv.nQueries)
 
-	// the 90ms timeout allows at most 4 retries
-	if numQueries > 4 {
-		t.Fatalf("Too many retries executed for query. Query executed %v times", numQueries)
-	}
-	// make sure query is retried to guard against regressions
-	if numQueries < 2 {
-		t.Fatalf("Not enough retries executed for query. Query executed %v times", numQueries)
+	// the 90ms timeout allows at most 4 retries but the maximum is 2 as per the retry policy
+	// the number of queries therefore needs to be 3 (initial query + 2 retries)
+	if numQueries != 3 {
+		t.Fatalf("Number of queries should be 3 but query executed %v times", numQueries)
 	}
 }
 

query_executor.go

@@ -48,7 +48,7 @@ func (q *queryExecutor) checkRetryPolicy(rq ExecutableQuery, err error) (RetryTy
 	if p.Attempt(rq) {
 		return p.GetRetryType(err), nil
 	}
-	return p.GetRetryType(err), err
+	return Rethrow, err
 }
 
 func (q *queryExecutor) executeQuery(qry ExecutableQuery) (*Iter, error) {