From 43c78ed7c6f0a5ec10127d914cf122a28c4cf37f Mon Sep 17 00:00:00 2001 From: Ivan Date: Wed, 8 Sep 2021 15:44:21 +0300 Subject: [PATCH] linting --- .golangci.yml | 67 +++++++++++++++++++++++++++++++++++++ cmd/reflector.go | 2 +- db/db.go | 37 -------------------- internal/metrics/metrics.go | 22 ++++++------ lite_db/db.go | 7 ++-- server/http/routes.go | 4 +-- server/http/worker.go | 4 +-- server/http3/client.go | 2 +- server/http3/server.go | 4 +-- server/http3/worker.go | 4 +-- server/peer/client.go | 2 +- server/peer/server.go | 10 +----- store/caching_test.go | 1 - store/gcache.go | 2 +- store/http.go | 26 +++++++------- store/ittt.go | 1 + store/noop.go | 2 +- store/s3.go | 1 - store/singleflight.go | 1 - wallet/network.go | 20 +++++------ 20 files changed, 121 insertions(+), 98 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..810eb82 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,67 @@ +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 5m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: true + + + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + skip-dirs: + - app/model + - app/migration + + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + skip-files: + - ".*\\.my\\.go$" + + + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners: false + +linters: + enable: + - govet + - unused + - govet + - errcheck + - unconvert + - gocyclo + - goimports + - varcheck + - golint + - nilerr + +linters-settings: + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 35 + revive: + # see https://github.com/mgechev/revive#available-rules for details. + ignore-generated-header: true + severity: warning + rules: + - name: indent-error-flow + severity: warning \ No newline at end of file diff --git a/cmd/reflector.go b/cmd/reflector.go index 017b9bf..5aa3363 100644 --- a/cmd/reflector.go +++ b/cmd/reflector.go @@ -167,7 +167,7 @@ func initUpstreamStore() store.BlobStore { Timeout: 30 * time.Second, }) case "http": - s = store.NewHttpStore(upstreamReflector) + s = store.NewHTTPStore(upstreamReflector) default: log.Fatalf("protocol is not recognized: %s", upstreamProtocol) } diff --git a/db/db.go b/db/db.go index 5242a0e..b57052a 100644 --- a/db/db.go +++ b/db/db.go @@ -659,43 +659,6 @@ func (s *SQL) GetStoredHashesInRange(ctx context.Context, start, end bits.Bitmap return } -// txFunc is a function that can be wrapped in a transaction -type txFunc func(tx *sql.Tx) error - -// withTx wraps a function in an sql transaction. the transaction is committed if there's no error, or rolled back if there is one. -// if dbOrTx is an sql.DB, a new transaction is started -func withTx(dbOrTx interface{}, f txFunc) (err error) { - var tx *sql.Tx - - switch t := dbOrTx.(type) { - case *sql.Tx: - tx = t - case *sql.DB: - tx, err = t.Begin() - if err != nil { - return err - } - defer func() { - if p := recover(); p != nil { - if rollBackError := tx.Rollback(); rollBackError != nil { - log.Error("failed to rollback tx on panic - ", rollBackError) - } - panic(p) - } else if err != nil { - if rollBackError := tx.Rollback(); rollBackError != nil { - log.Error("failed to rollback tx on panic - ", rollBackError) - } - } else { - err = errors.Err(tx.Commit()) - } - }() - default: - return errors.Err("db or tx required") - } - - return f(tx) -} - func closeRows(rows *sql.Rows) { if rows != nil { err := rows.Close() diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index d6a93fd..138c1ba 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -113,12 +113,12 @@ var ( Name: "peer_download_total", Help: "Total number of blobs downloaded from reflector through tcp protocol", }) - Http3DownloadCount = promauto.NewCounter(prometheus.CounterOpts{ + HTTP3DownloadCount = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "http3_blob_download_total", Help: "Total number of blobs downloaded from reflector through QUIC protocol", }) - HttpDownloadCount = promauto.NewCounter(prometheus.CounterOpts{ + HTTPDownloadCount = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "http_blob_download_total", Help: "Total number of blobs downloaded from reflector through HTTP protocol", @@ -154,7 +154,7 @@ var ( Name: "origin_requests_total", Help: "How many Get requests are in flight from the cache to the origin", }, []string{LabelCacheType, LabelComponent}) - //during thundering-herd situations, the metric below should be a lot smaller than the metric above + //nolint //during thundering-herd situations, the metric below should be a lot smaller than the metric above CacheWaitingRequestsCount = promauto.NewGaugeVec(prometheus.GaugeOpts{ Namespace: ns, Subsystem: subsystemCache, @@ -184,32 +184,32 @@ var ( Help: "Total number of SD blobs (and therefore streams) uploaded to reflector", }) - MtrInBytesTcp = promauto.NewCounter(prometheus.CounterOpts{ + MtrInBytesTCP = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "tcp_in_bytes", Help: "Total number of bytes downloaded through TCP", }) - MtrOutBytesTcp = promauto.NewCounter(prometheus.CounterOpts{ + MtrOutBytesTCP = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "tcp_out_bytes", Help: "Total number of bytes streamed out through TCP", }) - MtrInBytesUdp = promauto.NewCounter(prometheus.CounterOpts{ + MtrInBytesUDP = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "udp_in_bytes", Help: "Total number of bytes downloaded through UDP", }) - MtrInBytesHttp = promauto.NewCounter(prometheus.CounterOpts{ + MtrInBytesHTTP = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "http_in_bytes", Help: "Total number of bytes downloaded through HTTP", }) - MtrOutBytesUdp = promauto.NewCounter(prometheus.CounterOpts{ + MtrOutBytesUDP = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "udp_out_bytes", Help: "Total number of bytes streamed out through UDP", }) - MtrOutBytesHttp = promauto.NewCounter(prometheus.CounterOpts{ + MtrOutBytesHTTP = promauto.NewCounter(prometheus.CounterOpts{ Namespace: ns, Name: "http_out_bytes", Help: "Total number of bytes streamed out through UDP", @@ -229,12 +229,12 @@ var ( Name: "s3_in_bytes", Help: "Total number of incoming bytes (from S3-CF)", }) - Http3BlobReqQueue = promauto.NewGauge(prometheus.GaugeOpts{ + HTTP3BlobReqQueue = promauto.NewGauge(prometheus.GaugeOpts{ Namespace: ns, Name: "http3_blob_request_queue_size", Help: "Blob requests of https queue size", }) - HttpBlobReqQueue = promauto.NewGauge(prometheus.GaugeOpts{ + HTTPBlobReqQueue = promauto.NewGauge(prometheus.GaugeOpts{ Namespace: ns, Name: "http_blob_request_queue_size", Help: "Blob requests queue size of the HTTP protocol", diff --git a/lite_db/db.go b/lite_db/db.go index 6be9837..c53b320 100644 --- a/lite_db/db.go +++ b/lite_db/db.go @@ -1,4 +1,4 @@ -package lite_db +package lite_db //nolint import ( "database/sql" @@ -111,7 +111,10 @@ func (s *SQL) HasBlob(hash string) (bool, error) { // HasBlobs checks if the database contains the set of blobs and returns a bool map. func (s *SQL) HasBlobs(hashes []string) (map[string]bool, error) { exists, streamsNeedingTouch, err := s.hasBlobs(hashes) - s.touch(streamsNeedingTouch) + if err != nil { + return nil, err + } + err = s.touch(streamsNeedingTouch) return exists, err } diff --git a/server/http/routes.go b/server/http/routes.go index 56586b6..46e3891 100644 --- a/server/http/routes.go +++ b/server/http/routes.go @@ -67,9 +67,9 @@ func (s *Server) HandleGetBlob(c *gin.Context) { c.String(http.StatusInternalServerError, err.Error()) return } - metrics.MtrOutBytesHttp.Add(float64(len(blob))) + metrics.MtrOutBytesHTTP.Add(float64(len(blob))) metrics.BlobDownloadCount.Inc() - metrics.HttpDownloadCount.Inc() + metrics.HTTPDownloadCount.Inc() c.Header("Via", serialized) c.Header("Content-Disposition", "filename="+hash) c.Data(http.StatusOK, "application/octet-stream", blob) diff --git a/server/http/worker.go b/server/http/worker.go index 9b6fe3e..88667c7 100644 --- a/server/http/worker.go +++ b/server/http/worker.go @@ -28,7 +28,7 @@ func InitWorkers(server *Server, workers int) { case <-stopper.Ch(): case r := <-getReqCh: process(server, r) - metrics.HttpBlobReqQueue.Dec() + metrics.HTTPBlobReqQueue.Dec() } } }(i) @@ -36,7 +36,7 @@ func InitWorkers(server *Server, workers int) { } func enqueue(b *blobRequest) { - metrics.HttpBlobReqQueue.Inc() + metrics.HTTPBlobReqQueue.Inc() getReqCh <- b } diff --git a/server/http3/client.go b/server/http3/client.go index a92d53d..95101ce 100644 --- a/server/http3/client.go +++ b/server/http3/client.go @@ -114,7 +114,7 @@ func (c *Client) GetBlob(hash string) (stream.Blob, shared.BlobTrace, error) { blob := make([]byte, written) copy(blob, tmp.Bytes()) - metrics.MtrInBytesUdp.Add(float64(len(blob))) + metrics.MtrInBytesUDP.Add(float64(len(blob))) return blob, trace.Stack(time.Since(start), "http3"), nil } diff --git a/server/http3/server.go b/server/http3/server.go index 46fc12b..3ae36b8 100644 --- a/server/http3/server.go +++ b/server/http3/server.go @@ -208,7 +208,7 @@ func (s *Server) HandleGetBlob(w http.ResponseWriter, r *http.Request) { if err != nil { s.logError(err) } - metrics.MtrOutBytesUdp.Add(float64(len(blob))) + metrics.MtrOutBytesUDP.Add(float64(len(blob))) metrics.BlobDownloadCount.Inc() - metrics.Http3DownloadCount.Inc() + metrics.HTTP3DownloadCount.Inc() } diff --git a/server/http3/worker.go b/server/http3/worker.go index 563523e..cbbeb23 100644 --- a/server/http3/worker.go +++ b/server/http3/worker.go @@ -27,7 +27,7 @@ func InitWorkers(server *Server, workers int) { select { case <-stopper.Ch(): case r := <-getReqCh: - metrics.Http3BlobReqQueue.Dec() + metrics.HTTP3BlobReqQueue.Dec() process(server, r) } } @@ -36,7 +36,7 @@ func InitWorkers(server *Server, workers int) { } func enqueue(b *blobRequest) { - metrics.Http3BlobReqQueue.Inc() + metrics.HTTP3BlobReqQueue.Inc() getReqCh <- b } diff --git a/server/peer/client.go b/server/peer/client.go index fc4192a..bd80474 100644 --- a/server/peer/client.go +++ b/server/peer/client.go @@ -158,7 +158,7 @@ func (c *Client) GetBlob(hash string) (stream.Blob, shared.BlobTrace, error) { if err != nil { return nil, (*resp.RequestTrace).Stack(time.Since(start), "tcp"), err } - metrics.MtrInBytesTcp.Add(float64(len(blob))) + metrics.MtrInBytesTCP.Add(float64(len(blob))) return blob, trace.Stack(time.Since(start), "tcp"), nil } diff --git a/server/peer/server.go b/server/peer/server.go index 4061ea5..27c3ed9 100644 --- a/server/peer/server.go +++ b/server/peer/server.go @@ -277,7 +277,7 @@ func (s *Server) handleCompositeRequest(data []byte) ([]byte, error) { BlobHash: reflector.BlobHash(blob), Length: len(blob), } - metrics.MtrOutBytesTcp.Add(float64(len(blob))) + metrics.MtrOutBytesTCP.Add(float64(len(blob))) metrics.BlobDownloadCount.Inc() metrics.PeerDownloadCount.Inc() } @@ -368,14 +368,6 @@ type availabilityResponse struct { AvailableBlobs []string `json:"available_blobs"` } -type paymentRateRequest struct { - BlobDataPaymentRate float64 `json:"blob_data_payment_rate"` -} - -type paymentRateResponse struct { - BlobDataPaymentRate string `json:"blob_data_payment_rate"` -} - type blobRequest struct { RequestedBlob string `json:"requested_blob"` } diff --git a/store/caching_test.go b/store/caching_test.go index 6fd5544..c40e845 100644 --- a/store/caching_test.go +++ b/store/caching_test.go @@ -174,5 +174,4 @@ func (s *SlowBlobStore) Delete(hash string) error { } func (s *SlowBlobStore) Shutdown() { - return } diff --git a/store/gcache.go b/store/gcache.go index f21ed8d..c006166 100644 --- a/store/gcache.go +++ b/store/gcache.go @@ -59,7 +59,7 @@ func NewGcacheStore(component string, store BlobStore, maxSize int, strategy Evi } go func() { if lstr, ok := store.(lister); ok { - err := l.loadExisting(lstr, int(maxSize)) + err := l.loadExisting(lstr, maxSize) if err != nil { panic(err) // TODO: what should happen here? panic? return nil? just keep going? } diff --git a/store/http.go b/store/http.go index 8f9f403..43b72d0 100644 --- a/store/http.go +++ b/store/http.go @@ -16,23 +16,23 @@ import ( "github.com/lbryio/lbry.go/v2/stream" ) -// HttpStore is a store that works on top of the HTTP protocol -type HttpStore struct { +// HTTPStore is a store that works on top of the HTTP protocol +type HTTPStore struct { upstream string httpClient *http.Client } -func NewHttpStore(upstream string) *HttpStore { - return &HttpStore{ +func NewHTTPStore(upstream string) *HTTPStore { + return &HTTPStore{ upstream: "http://" + upstream, httpClient: getClient(), } } -const nameHttp = "http" +const nameHTTP = "http" -func (n *HttpStore) Name() string { return nameHttp } -func (n *HttpStore) Has(hash string) (bool, error) { +func (n *HTTPStore) Name() string { return nameHTTP } +func (n *HTTPStore) Has(hash string) (bool, error) { url := n.upstream + "/blob?hash=" + hash req, err := http.NewRequest("HEAD", url, nil) @@ -58,7 +58,7 @@ func (n *HttpStore) Has(hash string) (bool, error) { return false, errors.Err("upstream error. Status code: %d (%s)", res.StatusCode, string(body)) } -func (n *HttpStore) Get(hash string) (stream.Blob, shared.BlobTrace, error) { +func (n *HTTPStore) Get(hash string) (stream.Blob, shared.BlobTrace, error) { start := time.Now() url := n.upstream + "/blob?hash=" + hash @@ -95,7 +95,7 @@ func (n *HttpStore) Get(hash string) (stream.Blob, shared.BlobTrace, error) { blob := make([]byte, written) copy(blob, tmp.Bytes()) - metrics.MtrInBytesHttp.Add(float64(len(blob))) + metrics.MtrInBytesHTTP.Add(float64(len(blob))) return blob, trace.Stack(time.Since(start), n.Name()), nil } var body []byte @@ -106,16 +106,16 @@ func (n *HttpStore) Get(hash string) (stream.Blob, shared.BlobTrace, error) { return nil, trace.Stack(time.Since(start), n.Name()), errors.Err("upstream error. Status code: %d (%s)", res.StatusCode, string(body)) } -func (n *HttpStore) Put(string, stream.Blob) error { +func (n *HTTPStore) Put(string, stream.Blob) error { return shared.ErrNotImplemented } -func (n *HttpStore) PutSD(string, stream.Blob) error { +func (n *HTTPStore) PutSD(string, stream.Blob) error { return shared.ErrNotImplemented } -func (n *HttpStore) Delete(string) error { +func (n *HTTPStore) Delete(string) error { return shared.ErrNotImplemented } -func (n *HttpStore) Shutdown() {} +func (n *HTTPStore) Shutdown() {} // buffer pool to reduce GC // https://www.captaincodeman.com/2017/06/02/golang-buffer-pool-gotcha diff --git a/store/ittt.go b/store/ittt.go index 359503d..f469187 100644 --- a/store/ittt.go +++ b/store/ittt.go @@ -37,6 +37,7 @@ func (c *ITTTStore) Has(hash string) (bool, error) { return has, err } +// TODO: refactor error check, why return error? should we check if `err == nil` ? // Get tries to get the blob from this first, falling back to that. func (c *ITTTStore) Get(hash string) (stream.Blob, shared.BlobTrace, error) { start := time.Now() diff --git a/store/noop.go b/store/noop.go index fd7b35e..abb51b2 100644 --- a/store/noop.go +++ b/store/noop.go @@ -21,4 +21,4 @@ func (n *NoopStore) Get(_ string) (stream.Blob, shared.BlobTrace, error) { func (n *NoopStore) Put(_ string, _ stream.Blob) error { return nil } func (n *NoopStore) PutSD(_ string, _ stream.Blob) error { return nil } func (n *NoopStore) Delete(_ string) error { return nil } -func (n *NoopStore) Shutdown() { return } +func (n *NoopStore) Shutdown() {} diff --git a/store/s3.go b/store/s3.go index 00b3f95..0183f6e 100644 --- a/store/s3.go +++ b/store/s3.go @@ -166,5 +166,4 @@ func (s *S3Store) initOnce() error { // Shutdown shuts down the store gracefully func (s *S3Store) Shutdown() { - return } diff --git a/store/singleflight.go b/store/singleflight.go index fa8ae99..e2088e0 100644 --- a/store/singleflight.go +++ b/store/singleflight.go @@ -124,5 +124,4 @@ func (s *singleflightStore) putter(hash string, blob stream.Blob) func() (interf // Shutdown shuts down the store gracefully func (s *singleflightStore) Shutdown() { s.BlobStore.Shutdown() - return } diff --git a/wallet/network.go b/wallet/network.go index 10e4edf..a685af6 100644 --- a/wallet/network.go +++ b/wallet/network.go @@ -36,7 +36,7 @@ type response struct { type Node struct { transport *TCPTransport - nextId atomic.Uint32 + nextID atomic.Uint32 grp *stop.Group handlersMu *sync.RWMutex @@ -155,7 +155,7 @@ func (n *Node) listen() { return case bytes := <-n.transport.Responses(): msg := &struct { - Id uint32 `json:"id"` + ID uint32 `json:"id"` Method string `json:"method"` Error struct { Code int `json:"code"` @@ -163,7 +163,7 @@ func (n *Node) listen() { } `json:"error"` }{} msg2 := &struct { - Id uint32 `json:"id"` + ID uint32 `json:"id"` Method string `json:"method"` Error struct { Code int `json:"code"` @@ -181,7 +181,7 @@ func (n *Node) listen() { // maybe that happens because the wallet server passes a lbrycrd error through to us? if err2 := json.Unmarshal(bytes, msg2); err2 == nil { err = nil - msg.Id = msg2.Id + msg.ID = msg2.ID msg.Method = msg2.Method msg.Error = msg2.Error.Message } @@ -210,7 +210,7 @@ func (n *Node) listen() { } n.handlersMu.RLock() - c, ok := n.handlers[msg.Id] + c, ok := n.handlers[msg.ID] n.handlersMu.RUnlock() if ok { c <- r @@ -231,15 +231,15 @@ func (n *Node) listen() { // request makes a request to the server and unmarshals the response into v. func (n *Node) request(method string, params []string, v interface{}) error { msg := struct { - Id uint32 `json:"id"` + ID uint32 `json:"id"` Method string `json:"method"` Params []string `json:"params"` }{ - Id: n.nextId.Load(), + ID: n.nextID.Load(), Method: method, Params: params, } - n.nextId.Inc() + n.nextID.Inc() bytes, err := json.Marshal(msg) if err != nil { @@ -250,7 +250,7 @@ func (n *Node) request(method string, params []string, v interface{}) error { c := make(chan response, 1) n.handlersMu.Lock() - n.handlers[msg.Id] = c + n.handlers[msg.ID] = c n.handlersMu.Unlock() err = n.transport.Send(bytes) @@ -268,7 +268,7 @@ func (n *Node) request(method string, params []string, v interface{}) error { } n.handlersMu.Lock() - delete(n.handlers, msg.Id) + delete(n.handlers, msg.ID) n.handlersMu.Unlock() if r.err != nil { -- 2.45.3