-
Notifications
You must be signed in to change notification settings - Fork 115
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
go/runtime/client: add SubmitTxNoWait method #3746
Conversation
f8bc276
to
4ac0542
Compare
Codecov Report
@@ Coverage Diff @@
## master #3746 +/- ##
==========================================
+ Coverage 66.86% 66.88% +0.02%
==========================================
Files 401 401
Lines 39855 39881 +26
==========================================
+ Hits 26648 26675 +27
+ Misses 9415 9408 -7
- Partials 3792 3798 +6
Continue to review full report at Codecov.
|
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.
Cool, also moves the retry/submission logic into the "watcher".
81f89f9
to
f0a43bf
Compare
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 good to me, maybe @jberci can you take a quick look as well.
@@ -561,8 +550,8 @@ func (c *runtimeClient) Start() error { | |||
// Implements service.BackgroundService. | |||
func (c *runtimeClient) Stop() { | |||
// Watchers. | |||
for _, watcher := range c.watchers { | |||
watcher.Stop() | |||
for _, submitter := range c.txSubmitters { |
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.
Just noticed that this loop (and below) should probably be under a c.Lock()
guard as in theory a request could come in right when this is shutting down?
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.
nice find! yeah indeed should be, will update.
f0a43bf
to
c8b2cfd
Compare
c8b2cfd
to
37ca8ef
Compare
Fixes: #3444
TODO:
client/watcher.go
(nowclient/submitter.go
) it is not just a watcher anymore but also submits transactions. Also removed the implementation ofBackgroundService
trait in the submitter for some minor simplifications - since this is aruntime/client
package private struct only used by the client (which remains aBackgroundService
) i think that's fine.