We have this one solitary metric from the go-metrics package. In principle
this statistic could be useful, but the way we have it hooked up, nothing can
observe the value: We don't export it, we don't log it, and it does not auto
publish anywhere.
Given that this state of affairs has not changed since the metric was first
added in 2017 (c08618f), I think we can safely discard it. No one is now or has
ever gotten any data out of this metric.
* rpc/client: remove the placeholder RunState type.
I added the RunState type in #6971 to disconnect clients from the service
plumbing, which they do not need. Now that we have more complete context
plumbing, the lifecycle of a client no longer depends on this type: It serves
as a carrier for a logger, and a Boolean flag for "running" status, neither of
which is used outside of tests.
Logging in particular is defaulted to a no-op logger in all production use.
Arguably we could just remove the logging calls, since they are never invoked
except in tests. To defer the question of whether we should do that or make the
logging go somewhere more productive, I've preserved the existing use here.
Remove use of the IsRunning method that was provided by the RunState, and use
the Start method and context to govern client lifecycle.
Remove the one test that exercised "unstarted" clients. I would like to remove
that method entirely, but that will require updating the constructors for all
the client types to plumb a context and possibly other options. I have deferred
that for now.
* rpc: simplify the handling of JSON-RPC request and response IDs
Replace the ID wrapper interface with plain JSON. Internally, the client
libraries use only integer IDs, and the server does not care about the ID
structure apart from checking its validity.
Basic structure of this change:
- Remove the jsonrpcid interface and its helpers.
- Unexport the ID field of request and response.
- Add helpers for constructing requests and responses.
- Fix up usage and tests.
These are only ever used with the defaults, except in our own tests. A search
of cs.github.com shows no other callers.
The use in the test was solely to bug out the go-metrics package so its
goroutines don't trigger the leak checker. Use the package's own flag for that
purpose instead. Note that calling "Stop" on the metric helps, but is not
sufficient -- the Stop does not wait for its goroutine to exit.
This continues the push of plumbing contexts through tendermint. I
attempted to find all goroutines in the production code (non-test) and
made sure that these threads would exit when their contexts were
canceled, and I believe this PR does that.
The code in the Tendermint repository makes heavy use of import aliasing.
This is made necessary by our extensive reuse of common base package names, and
by repetition of similar names across different subdirectories.
Unfortunately we have not been very consistent about which packages we alias in
various circumstances, and the aliases we use vary. In the spirit of the advice
in the style guide and https://github.com/golang/go/wiki/CodeReviewComments#imports,
his change makes an effort to clean up and normalize import aliasing.
This change makes no API or behavioral changes. It is a pure cleanup intended
o help make the code more readable to developers (including myself) trying to
understand what is being imported where.
Only unexported names have been modified, and the changes were generated and
applied mechanically with gofmt -r and comby, respecting the lexical and
syntactic rules of Go. Even so, I did not fix every inconsistency. Where the
changes would be too disruptive, I left it alone.
The principles I followed in this cleanup are:
- Remove aliases that restate the package name.
- Remove aliases where the base package name is unambiguous.
- Move overly-terse abbreviations from the import to the usage site.
- Fix lexical issues (remove underscores, remove capitalization).
- Fix import groupings to more closely match the style guide.
- Group blank (side-effecting) imports and ensure they are commented.
- Add aliases to multiple imports with the same base package name.
## Description
Internalize some libs. This reduces the amount ot public API tendermint is supporting. The moved libraries are mainly ones that are used within Tendermint-core.
## Description
This PR wraps the stdlib sync.(RW)Mutex & godeadlock.(RW)Mutex. This enables using go-deadlock via a build flag instead of using sed to replace sync with godeadlock in all files
Closes: #3242
## Description
partially cleanup in preparation for errcheck
i ignored a bunch of defer errors in tests but with the update to go 1.14 we can use `t.Cleanup(func() { if err := <>; err != nil {..}}` to cover those errors, I will do this in pr number two of enabling errcheck.
ref #5059
* format: add format cmd & goimport repo
- replaced format command
- added goimports to format command
- ran goimports
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* fix outliers & undo proto file changes
https://www.jsonrpc.org/specification
What is done in this PR:
JSONRPCClient: validate that Response.ID matches Request.ID I wanted
to do the same for the WSClient, but since we're sending events as
responses, not notifications, checking IDs would require storing
them in memory indefinitely (and we won't be able to remove them
upon client unsubscribing because ID is different then).
Request.ID is now optional. Notification is a Request without an ID.
Previously "" or 0 were considered as notifications
Remove #event suffix from ID from an event response (partially fixes
#2949) ID must be either string, int or null AND must be equal to
request's ID. Now, because we've implemented events as responses, WS
clients are tripping when they see Response.ID("0#event") !=
Request.ID("0"). Implementing events as requests would require a lot
of time (~ 2 days to completely rewrite WS client and server)
generate unique ID for each request
switch to integer IDs instead of "json-client-XYZ"
id=0 method=/subscribe
id=0 result=...
id=1 method=/abci_query
id=1 result=...
> send events (resulting from /subscribe) as requests+notifications (not
responses)
this will require a lot of work. probably not worth it
* rpc: generate an unique ID for each request
in conformance with JSON-RPC spec
* WSClient: check for unsolicited responses
* fix golangci warnings
* save commit
* fix errors
* remove ID from responses from subscribe
Refs #2949
* clients are safe for concurrent access
* tm-bench: switch to int ID
* fixes after my own review
* comment out sentIDs in WSClient
see commit body for the reason
* remove body.Close
it will be closed automatically
* stop ws connection outside of write/read routines
also, use t.Rate in tm-bench indexer when calculating ID
fix gocritic issues
* update swagger.yaml
* Apply suggestions from code review
* fix stylecheck and golint linter warnings
* update changelog
* update changelog2
cleanup to add linter
grpc change:
https://godoc.org/google.golang.org/grpc#WithContextDialerhttps://godoc.org/google.golang.org/grpc#WithDialer
grpc/grpc-go#2627
prometheous change:
due to UninstrumentedHandler, being deprecated in the future
empty branch = empty if or else statement
didn't delete them entirely but commented
couldn't find a reason to have them
could not replicate the issue #3406
but if want to keep it commented then we should comment out the if statement as well
* Updated code with feedback from @melekes, @ebuchman and @silasdavis.
* Added Makefile clause `release` to only run the test on seeing tag
`release` during releases i.e
```shell
make release
```
which will run the comprehensive and long integration-ish tests.
Fixes https://github.com/tendermint/tendermint/issues/751.
Adds jitter to our exponential backoff to mitigate a self DDOS
vector. The jitter is a randomly picked percentage of a second
whose purpose is to ensure that each exponential backoff retry
occurs within (1<<attempts) == 2**attempts, but with the delay
each client will have a random buffer time before it tries to
reconnect instead of all at once reconnections that might even
bring back the previous conditions that might have caused the
dial to the WSServer to have failed e.g
* Network outage
* File descriptor exhaustion
* False positives from firewalls
etc
server:
- always has read & write timeouts
- ping handler never blocks the reader (see A)
- sends regular pings to check up on a client
A:
at some point server write buffer can become full, so in order not to
block reads from a client (see
https://github.com/gorilla/websocket/issues/97), server may skip some
pongs. As a result, client may disconnect. But you either have to do
that or block the reader. There is no third way.
client:
- optional read & write timeouts
- optional ping/pong to measure latency
Callum Waters
M. J. Fromberger
Sam Kleinman
Marko
Anton Kaliaev
Greg Szabo
Peter Mrekaj
Phil Salant
Zach Ramsay
Ethan Buchman
Liamsi
Emmanuel Odeke
Silas Davis
Alexandre Thibault