Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Internal] Performance: Fixes exception serialization when tracing is not enabled #1999

Merged
merged 3 commits into from
Nov 13, 2020

Conversation

ealsur
Copy link
Member

@ealsur ealsur commented Nov 13, 2020

When an Exception travels through the ResourceThrottleRetryPolicy.ShouldRetry, and the error is not one the current policy handles, we trace it like so:

DefaultTrace.TraceError(
        "Operation will NOT be retried. Current attempt {0}, Exception: {1} ",
        this.currentAttemptCount,
        this.GetExceptionMessage(exception));

DefaultTrace.TraceError has this signature:

TraceError(string format, params object[] args)

So the object[] ToString invocation on each of the arguments is only called if tracing is enabled.

But our current code on this.GetExceptionMessage calls exception.ToString() so it is serializing the exception even if tracing is not enabled (because it needs to construct the object array).

This PR defers the ToString call to only be invoked if there is tracing enabled.

Normal 429s (Throttles) are exceptionless, so this issue surfaces only in cases where there is another exception happening on the transport path.

@ealsur ealsur merged commit 91ed21c into master Nov 13, 2020
@ealsur ealsur deleted the users/ealsur/errorpolicy branch November 13, 2020 02:30
@ghost
Copy link

ghost commented Dec 15, 2021

Closing due to in-activity, pease feel free to re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants