Enhance Proof Courier Handlers with Lazy Connection Attempts for HashMail and UniverseRPC #1092
Conversation
Pull Request Test Coverage Report for Build 10506105341Details
💛 - Coveralls |
ffranr
force-pushed
the
check-proof-courier-con
branch
from
cd2dd0f to
afe2b87
Compare
ffranr
changed the title
ffranr
force-pushed
the
check-proof-courier-con
branch
2 times, most recently
from
81353e7 to
9a021a1
Compare
This commit updates the HashMail proof courier handler to make connection attempts optionally lazy. Connection attempts are now integrated into the backoff procedure where feasible, improving the robustness of the connection handling process.
ffranr
force-pushed
the
check-proof-courier-con
branch
from
9a021a1 to
9756424
Compare
There was a problem hiding this comment.
Seems solid! Few comments:
- Would be good for the initial sentence for commit 2 to be different from commit 1.
- What's the recommended setup once this PR is in; should
lazyConnectalways be true?
From itests passing (including those using backoff) this is covered in testing, which is nice.
| const ( | ||
| // CourierConnStatusUnknown indicates that the connection status is | ||
| // unknown. | ||
| CourierConnStatusUnknown CourierConnStatus = iota |
There was a problem hiding this comment.
non-blocking nit: may be good to have unknown after disconnected, to follow the enum pattern we have elsewhere / make comparison a bit safer.
Though with exhaustive switches I suppose the order doesn't matter.
There was a problem hiding this comment.
I think it's a good idea to have unknown as the first enum value, ensuring that the default for a CourierConnStatus variable is unknown. Which means that if we don't set the field, it won't default to something opinionated.
|
|
||
| // At this point, we know that the connection is not ready. We'll now | ||
| // attempt to establish a new connection to the courier service. | ||
| dialOpts, err := serverDialOpts() |
There was a problem hiding this comment.
You mentioned offline that you wanted the option for the connection setup to be blocking; I think adding the WithBlock() grpc.DialOption would work for that.
https://pkg.go.dev/google.golang.org/[email protected]#WithBlock
There was a problem hiding this comment.
That's what I've been playing around with in conjunction with DialContext 👍
@jharveyb I'm not sure what you mean. Should I make them different for clarity? I think you want the commit subject to be different. I've changes the second commit slightly.
|
ffranr
force-pushed
the
check-proof-courier-con
branch
from
9756424 to
cbe2e2d
Compare
This commit modifies the Universe RPC proof courier handler to allow connection attempts to be optionally lazy. Connection attempts are now integrated into the backoff procedure where feasible, improving the robustness of the connection handling process.
ffranr
force-pushed
the
check-proof-courier-con
branch
from
cbe2e2d to
e514015
Compare
This PR introduces updates to the HashMail and UniverseRPC proof courier handlers, specifically making connection attempts optionally lazy. The changes are aimed at enhancing the robustness of the connection handling process by integrating these attempts into the backoff procedure where feasible.