DB Retries #860
DB Retries #860
Conversation
| @@ -1,3 +1,4 @@ | |||
| #![recursion_limit = "256"] | |||
There was a problem hiding this comment.
Needing this is definitely a smell that we are doing something crazy with macros. It fails to compile without raising the limit saying we are at 130.
There was a problem hiding this comment.
crossing the recursion limit first showed up after adding tracing::instrument attribute macros everywhere for benchmarking. These macros are nested within each other depending on the codepath resulting in us reaching the default limit. The easiest is to just remove the tracing::instrument macros. I haven't tried it but we can also specify a static verbosity level to skip compiling these macros unless we specify a trace level at compile time. This would probably get rid of the attribute for the xmtp_mls crate, but would require the attribute for benchmarks or anything making use of the instrumentation.
| @@ -19,6 +22,12 @@ pub enum WrappedApiError { | |||
| AssociationDeserialization(#[from] AssociationDeserializationError), | |||
| } | |||
|
|
|||
| impl RetryableError for WrappedApiError { | |||
| fn is_retryable(&self) -> bool { | |||
| matches!(self, Self::Api(_)) | |||
There was a problem hiding this comment.
API errors werent being retried either
| @@ -146,6 +146,8 @@ impl EncryptedMessageStore { | |||
| conn.batch_execute(&format!("PRAGMA key = \"x'{}'\";", hex::encode(key)))?; | |||
| } | |||
|
|
|||
| conn.batch_execute("PRAGMA busy_timeout = 5000;")?; | |||
There was a problem hiding this comment.
Are we seeing busy errors that take more than 5s?
There was a problem hiding this comment.
Tbh, this change did nothing. We aren't seeing busy errors at all, since the DB locks are a different error code altogether.
tl;dr
retryable!macro to not reference the error returned. A double reference will fall back to the genericRetryableErrorimplementation for&T, instead of using the user-defined implementationself