Skip to content

Commit 6f45cba

Browse files
wprzytulacvybhu
authored andcommitted
Fixed bug: inverted bool in DefaultRetryPolicy
The meaning of `data_present` was understood as the opposite in the code.
1 parent e484adc commit 6f45cba

File tree

1 file changed

+15
-14
lines changed

1 file changed

+15
-14
lines changed

scylla/src/transport/retry_policy.rs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ impl RetrySession for DefaultRetrySession {
163163
}
164164
// ReadTimeout - coordinator didn't receive enough replies in time.
165165
// Retry at most once and only if there were actually enough replies
166-
// to satisfy consistency but they were all just checksums (data_present == true).
166+
// to satisfy consistency but they were all just checksums (data_present == false).
167167
// This happens when the coordinator picked replicas that were overloaded/dying.
168168
// Retried request should have some useful response because the node will detect
169169
// that these replicas are dead.
@@ -176,7 +176,7 @@ impl RetrySession for DefaultRetrySession {
176176
},
177177
_,
178178
) => {
179-
if !self.was_read_timeout_retry && received >= required && *data_present {
179+
if !self.was_read_timeout_retry && received >= required && !*data_present {
180180
self.was_read_timeout_retry = true;
181181
RetryDecision::RetrySameNode
182182
} else {
@@ -374,61 +374,62 @@ mod tests {
374374
// On ReadTimeout we retry one time if there were enough responses and the data was present no matter the idempotence
375375
#[test]
376376
fn default_read_timeout() {
377-
// Enough responses and data_present == true
378-
let enough_responses_with_data = QueryError::DbError(
377+
// Enough responses and data_present == false - coordinator received only checksums
378+
let enough_responses_no_data = QueryError::DbError(
379379
DbError::ReadTimeout {
380380
consistency: LegacyConsistency::Regular(Consistency::Two),
381381
received: 2,
382382
required: 2,
383-
data_present: true,
383+
data_present: false,
384384
},
385385
String::new(),
386386
);
387387

388388
// Not idempotent
389389
let mut policy = DefaultRetryPolicy::new().new_session();
390390
assert_eq!(
391-
policy.decide_should_retry(make_query_info(&enough_responses_with_data, false)),
391+
policy.decide_should_retry(make_query_info(&enough_responses_no_data, false)),
392392
RetryDecision::RetrySameNode
393393
);
394394
assert_eq!(
395-
policy.decide_should_retry(make_query_info(&enough_responses_with_data, false)),
395+
policy.decide_should_retry(make_query_info(&enough_responses_no_data, false)),
396396
RetryDecision::DontRetry
397397
);
398398

399399
// Idempotent
400400
let mut policy = DefaultRetryPolicy::new().new_session();
401401
assert_eq!(
402-
policy.decide_should_retry(make_query_info(&enough_responses_with_data, true)),
402+
policy.decide_should_retry(make_query_info(&enough_responses_no_data, true)),
403403
RetryDecision::RetrySameNode
404404
);
405405
assert_eq!(
406-
policy.decide_should_retry(make_query_info(&enough_responses_with_data, true)),
406+
policy.decide_should_retry(make_query_info(&enough_responses_no_data, true)),
407407
RetryDecision::DontRetry
408408
);
409409

410-
// Enough responses but data_present == false
411-
let enough_responses_no_data = QueryError::DbError(
410+
// Enough responses but data_present == true - coordinator probably timed out
411+
// waiting for read-repair acknowledgement.
412+
let enough_responses_with_data = QueryError::DbError(
412413
DbError::ReadTimeout {
413414
consistency: LegacyConsistency::Regular(Consistency::Two),
414415
received: 2,
415416
required: 2,
416-
data_present: false,
417+
data_present: true,
417418
},
418419
String::new(),
419420
);
420421

421422
// Not idempotent
422423
let mut policy = DefaultRetryPolicy::new().new_session();
423424
assert_eq!(
424-
policy.decide_should_retry(make_query_info(&enough_responses_no_data, false)),
425+
policy.decide_should_retry(make_query_info(&enough_responses_with_data, false)),
425426
RetryDecision::DontRetry
426427
);
427428

428429
// Idempotent
429430
let mut policy = DefaultRetryPolicy::new().new_session();
430431
assert_eq!(
431-
policy.decide_should_retry(make_query_info(&enough_responses_no_data, true)),
432+
policy.decide_should_retry(make_query_info(&enough_responses_with_data, true)),
432433
RetryDecision::DontRetry
433434
);
434435

0 commit comments

Comments
 (0)