Skip to content

Commit

Permalink
refactor(rpc): allocate PingPongLatencyTimer on start (backport comet…
Browse files Browse the repository at this point in the history
…bft#2804) (cometbft#2813)

Followup to cometbft#2792 which closed cometbft#2771.

cometbft#2792 does not handle the case where Start is never called. If Start is
not called, Stop returns an error, thus with cometbft#2792's implementation the
only way to ensure that PingPongLatencyTimer is cleaned up is to call
Start and Stop, even when not using any of the features provided by
Start (i.e. events).

This PR moves initialization of PingPongLatencyTimer into OnStart so
that it is only initialized if it is going to be used. This PR also
moves cleanup of PingPongLatencyTimer into readRoutine's defer statement
to align it with other cleanup (i.e. closing the connection).

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2804 done by
[Mergify](https://mergify.com).

Co-authored-by: Ethan Reesor <firelizzard@gmail.com>
  • Loading branch information
mergify[bot] and firelizzard18 authored Apr 15, 2024
1 parent de17c2d commit 365d4e4
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions rpc/jsonrpc/client/ws_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,11 @@ func NewWS(remoteAddr, endpoint string, options ...func(*WSClient)) (*WSClient,
}

c := &WSClient{
Address: parsedURL.GetTrimmedHostWithPath(),
Username: username,
Password: password,
Dialer: dialFn,
Endpoint: endpoint,
PingPongLatencyTimer: metrics.NewTimer(),
Address: parsedURL.GetTrimmedHostWithPath(),
Username: username,
Password: password,
Dialer: dialFn,
Endpoint: endpoint,

maxReconnectAttempts: defaultMaxReconnectAttempts,
readWait: defaultReadWait,
Expand Down Expand Up @@ -190,6 +189,7 @@ func (c *WSClient) OnStart() error {
}

c.ResponsesCh = make(chan types.RPCResponse)
c.PingPongLatencyTimer = metrics.NewTimer()

c.send = make(chan types.RPCRequest)
// 1 additional error may come from the read/write
Expand All @@ -213,7 +213,6 @@ func (c *WSClient) Stop() error {
}
// only close user-facing channels when we can't write to them
c.wg.Wait()
c.PingPongLatencyTimer.Stop()
close(c.ResponsesCh)

return nil
Expand Down Expand Up @@ -473,6 +472,7 @@ func (c *WSClient) readRoutine() {
// ignore error; it will trigger in tests
// likely because it's closing an already closed connection
// }
c.PingPongLatencyTimer.Stop()
c.wg.Done()
}()

Expand Down

0 comments on commit 365d4e4

Please sign in to comment.