Skip to content

Commit ad1b8d7

Browse files
p-mongop
authored andcommitted
Fix RUBY-2004 Indicated attempt not always correct (#1568)
* RUBY-2004 Use do-end syntax for expect blocks * RUBY-2004 Refactor for generality * RUBY-2004 Indicate whether modern or legacy retry logic is used * RUBY-2004 Organize more * RUBY-2004 Fix incorrect operation attempt numbering * RUBY-2004 Speed up the tests * RUBY-2004 Test retryable writes * RUBY-2004 Indicate when retries are disabled * RUBY-2004 Explicitly request modern behavior because some test confgurations use legacy retries * RUBY-2004 Need wired tiger for modern retries * RUBY-2004 Misc cleanup
1 parent bcd157d commit ad1b8d7

File tree

4 files changed

+273
-83
lines changed

4 files changed

+273
-83
lines changed

lib/mongo/retryable.rb

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,12 @@ def read_with_retry(session = nil, server_selector = nil, &block)
119119
legacy_read_with_retry(session, server_selector, &block)
120120
else
121121
server = select_server(cluster, server_selector, session)
122-
yield server
122+
begin
123+
yield server
124+
rescue Error::SocketError, Error::SocketTimeoutError, Error::OperationFailure => e
125+
e.add_note('retries disabled')
126+
raise e
127+
end
123128
end
124129
end
125130

@@ -214,12 +219,14 @@ def write_with_retry(session, write_concern, ending_transaction = false, &block)
214219
begin
215220
yield(server, txn_num, false)
216221
rescue Error::SocketError, Error::SocketTimeoutError => e
222+
e.add_note('modern retry')
217223
e.add_note("attempt 1")
218224
if session.in_transaction? && !ending_transaction
219225
raise e
220226
end
221227
retry_write(e, session, txn_num, &block)
222228
rescue Error::OperationFailure => e
229+
e.add_note('modern retry')
223230
e.add_note("attempt 1")
224231
if e.unsupported_retryable_write?
225232
raise_unsupported_error(e)
@@ -250,7 +257,12 @@ def write_with_retry(session, write_concern, ending_transaction = false, &block)
250257
def nro_write_with_retry(session, write_concern, &block)
251258
if session && session.client.options[:retry_writes]
252259
server = select_server(cluster, ServerSelector.primary, session)
253-
yield server
260+
begin
261+
yield server
262+
rescue Error::SocketError, Error::SocketTimeoutError, Error::OperationFailure => e
263+
e.add_note('retries disabled')
264+
raise e
265+
end
254266
else
255267
legacy_write_with_retry(nil, session, &block)
256268
end
@@ -280,7 +292,8 @@ def legacy_write_with_retry(server = nil, session = nil)
280292
server ||= select_server(cluster, ServerSelector.primary, session)
281293
yield server
282294
rescue Error::OperationFailure => e
283-
e.add_note("attempt #{attempt + 1}")
295+
e.add_note('legacy retry')
296+
e.add_note("attempt #{attempt}")
284297
server = nil
285298
if attempt > client.max_write_retries
286299
raise e
@@ -298,18 +311,19 @@ def legacy_write_with_retry(server = nil, session = nil)
298311
private
299312

300313
def modern_read_with_retry(session, server_selector, &block)
301-
attempt = 0
302314
server = select_server(cluster, server_selector, session)
303315
begin
304316
yield server
305317
rescue Error::SocketError, Error::SocketTimeoutError => e
306-
e.add_note("attempt #{attempt + 1}")
318+
e.add_note('modern retry')
319+
e.add_note("attempt 1")
307320
if session.in_transaction?
308321
raise e
309322
end
310323
retry_read(e, server_selector, session, &block)
311324
rescue Error::OperationFailure => e
312-
e.add_note("attempt #{attempt + 1}")
325+
e.add_note('modern retry')
326+
e.add_note("attempt 1")
313327
if session.in_transaction? || !e.write_retryable?
314328
raise e
315329
end
@@ -324,15 +338,17 @@ def legacy_read_with_retry(session, server_selector)
324338
attempt += 1
325339
yield server
326340
rescue Error::SocketError, Error::SocketTimeoutError => e
327-
e.add_note("attempt #{attempt + 1}")
341+
e.add_note('legacy retry')
342+
e.add_note("attempt #{attempt}")
328343
if attempt > client.max_read_retries || (session && session.in_transaction?)
329344
raise e
330345
end
331346
log_retry(e, message: 'Legacy read retry')
332347
server = select_server(cluster, server_selector, session)
333348
retry
334349
rescue Error::OperationFailure => e
335-
e.add_note("attempt #{attempt + 1}")
350+
e.add_note('legacy retry')
351+
e.add_note("attempt #{attempt}")
336352
if e.retryable? && !(session && session.in_transaction?)
337353
if attempt > client.max_read_retries
338354
raise e
@@ -375,16 +391,19 @@ def retry_read(original_error, server_selector, session, &block)
375391
begin
376392
yield server, true
377393
rescue Error::SocketError, Error::SocketTimeoutError => e
394+
e.add_note('modern retry')
378395
e.add_note("attempt 2")
379396
raise e
380397
rescue Error::OperationFailure => e
398+
e.add_note('modern retry')
381399
unless e.write_retryable?
382400
original_error.add_note("later retry failed: #{e.class}: #{e}")
383401
raise original_error
384402
end
385403
e.add_note("attempt 2")
386404
raise e
387405
rescue => e
406+
e.add_note('modern retry')
388407
original_error.add_note("later retry failed: #{e.class}: #{e}")
389408
raise original_error
390409
end
@@ -398,15 +417,19 @@ def retry_write(original_error, session, txn_num, &block)
398417
# server unknown). Here we just need to wait for server selection.
399418
server = select_server(cluster, ServerSelector.primary, session)
400419
unless server.retry_writes?
420+
# Do not need to add "modern retry" here, it should already be on
421+
# the first exception.
401422
original_error.add_note('did not retry because server selected for retry does not supoprt retryable writes')
402423
raise original_error
403424
end
404425
log_retry(original_error, message: 'Write retry')
405426
yield(server, txn_num, true)
406427
rescue Error::SocketError, Error::SocketTimeoutError => e
428+
e.add_note('modern retry')
407429
e.add_note('attempt 2')
408430
raise e
409431
rescue Error::OperationFailure => e
432+
e.add_note('modern retry')
410433
if e.write_retryable?
411434
e.add_note('attempt 2')
412435
raise e
@@ -415,6 +438,8 @@ def retry_write(original_error, session, txn_num, &block)
415438
raise original_error
416439
end
417440
rescue => e
441+
# Do not need to add "modern retry" here, it should already be on
442+
# the first exception.
418443
original_error.add_note("later retry failed: #{e.class}: #{e}")
419444
raise original_error
420445
end

0 commit comments

Comments
 (0)