759038 - retry the query when the connection is invalidated

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[Ryan Kelly [:rfkelly]]

Attachment: sync2.0 patch to retry on connection error

SyncServer and AITC should re-try the query if they receive a disconnection error from the database.

This basically extends the functionality from Bug 692789 so that it covers more types of error, including (2006, "MySQL server has gone away") and (2013, "Lost connection to MySQL server during query).  The error codes that SQLAlchemy currently identifies as "connection_invalidated" for MySQL are (2006, 2013, 2014, 2045, 2055).

I'm attaching patches for both sync1.1/server-core, which has Bug 692789 in production but not on tip, and for sync2.0/AITC which did not include this fix when they were ported to pyramid.

Comment 1

[Ryan Kelly [:rfkelly]]

Attachment: server-core patch to retry on connection error

Comment 2

[:Atoll]

Wow, that's a big list of invalidated.

Does this retry *once*, or *more than once*, before stack tracing and 50x'ing the user request?  It's important that it be once, only.

Comment 3

[Ryan Kelly [:rfkelly]]

(In reply to Richard Soderberg [:atoll] from comment #2)
> Does this retry *once*, or *more than once*, before stack tracing and
> 50x'ing the user request?  It's important that it be once, only.

Each incoming request will attempt to execute a query, retry it once if that fails, then error out with a 50x if that fails a second time.

Comment 4

[:Atoll]

Okay. Seems reasonable to me. I don't suppose it's possible to prepend "/* retried */" so we can see them in the process list? But that's not a requirement, just a wishful.

Comment 5

[Ryan Kelly [:rfkelly]]

http://hg.mozilla.org/services/server-core/rev/b1577aa8ccf6

https://github.com/mozilla-services/server-syncstorage/commit/7a55430b198950162f7af36877bdb7e499f56a3b

Comment 6

[Ryan Kelly [:rfkelly]]

Attachment: fix to connection-invalidation detection

During loadtesting of AITC I discovered a problem with the previous patch - TimeoutError does not have a "connection_invalidated" attribute.  The attached fix restricts the innermost try-except to catch only those exceptions that have such an attribute.

Comment 7

[Ryan Kelly [:rfkelly]]

http://hg.mozilla.org/services/server-core/rev/80f5b9a04d46

https://github.com/mozilla-services/server-syncstorage/commit/2eeb34341beb115d63a8b3c8096ee79647d9aede

Comment 8

[James Bonacci [:jbonacci]]

Looks like this needs to be verified for Sync 1.1, 2.0, and AITC.