-
Notifications
You must be signed in to change notification settings - Fork 179
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
[Access] Cleanup access connection management #4730
[Access] Cleanup access connection management #4730
Conversation
@@ -378,9 +378,6 @@ func (b *backendScripts) tryExecuteScriptOnArchiveNode( | |||
}(closer) | |||
resp, err := archiveClient.ExecuteScriptAtBlockID(ctx, req) | |||
if err != nil { | |||
if status.Code(err) == codes.Unavailable { | |||
b.connFactory.InvalidateAccessAPIClient(executorAddress) | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
archiveClient
is using the connection manager, so this invalidation will happen automatically
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #4730 +/- ##
==========================================
- Coverage 55.68% 55.68% -0.01%
==========================================
Files 934 934
Lines 86532 86508 -24
==========================================
- Hits 48188 48173 -15
+ Misses 34707 34706 -1
+ Partials 3637 3629 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much cleaner now.
The client connection caching logic was updated to handle invalidations within a connection interceptor:
flow-go/engine/access/rpc/connection/manager.go
Lines 295 to 314 in 5c927af
The old
Invalidate*
methods were left, but mostly unused outside of tests.This PR removes those methods, fixes tests that were still using them, and fixes some other tests that were using an old version of the connection factory mock.