server: refactor process*Payload functions #644
Conversation
server/service.go
Outdated
| requestCtxCancel() | ||
| *result = *responsePayload | ||
| resultCh <- responsePayload | ||
| log.Info("received payload from relay") |
There was a problem hiding this comment.
I think there could be value in adding back a cancellation check here. There's also a check at the beginning of the function, but this one (closer to the end) is most likely to catch it, right? This would help prevent multiple, relatively verbose "received payload from relay" messages.
There was a problem hiding this comment.
I added back a check that should get rid of multiple received payload from relay messages, but its kinda ugly. I kinda prefer the multiple log lines, ptal
server/service.go
Outdated
| wg.Wait() | ||
| close(resultCh) | ||
| result := <-resultCh |
There was a problem hiding this comment.
This is a strange pattern, but it should work as intended. We still need the wait group unfortunately. Normally, reading from a channel is a blocking operation, but if the channel is closed it's not blocking. It would be cool if we could figure out a way of handling this better, where we don't necessarily have to wait for each request to finish. As in, remove the wait group and proceed as soon as we receive the first request.
There was a problem hiding this comment.
I removed the wg and changed the pattern so that we only need one request to finish / or the timeout to trigger
|
Somethings wrong... am investigating |
MariusVanDerWijden
force-pushed
the
refactor-process-payload
branch
from
d169095
to
ebb5790
Compare
|
Seems like the issue was the same as in #650 |
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #644 +/- ##
===========================================
- Coverage 43.84% 43.69% -0.16%
===========================================
Files 15 15
Lines 1608 1609 +1
===========================================
- Hits 705 703 -2
- Misses 847 849 +2
- Partials 56 57 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
MariusVanDerWijden
force-pushed
the
refactor-process-payload
branch
from
16c8e04
to
4f970ca
Compare
This PR changes the way the process functions work. Since it is safe to call
cancel()multiple times, and we only need one valid payload from the relays, I think its safe to stick the results in a channel and only query one of them