Update code to fix almost all pre-existing lint errors (#2008)

* Cleanup linting errors around deadcode

To be specific ineffassign, deadcode and unused errors

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Replaced by keysFn and if the config defines it then it will distribute
keys using hashing

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Hold over from when readBatch as also the iterator

Now that we have an iterator there is no need to also have a consumed
bool on the underlying object.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Necessary to fix the false sharing problem

Will never actually be used. Only necessary to pad out CPU cache lines.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Removing unused code

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Cleanup all gosimple suggestions

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Fixing all errcheck

Attempted not to change any existing logic so if an error was ignored we
will now either explicitly ignore it or in the case of a goroutine being
called with a func that is ignoring the error we will just put a
//noling:errcheck on that line.

If it was in a test case we went ahead and did the extra assertion
checks b/c its always good to know where something might have errored in
your test cases.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Fix most staticcheck lint issues

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Cleanup deprecated call to snappy.NewWriter

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Remove rev from Makefile

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Reorder imports

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Ignoring this for now

We have opened up issue
cortexproject/cortex#2015 to address the
deprecation of this type.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Explicitly ignoring this error

The test is currently failing due to a data race. I believe it is due to
this bug in golang.

golang/go#30597

As far as this test cares it does not really matter that this happens so
removing the need to check for NoError "fixes" it.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Require noerror since this is a test

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Switch over to use require.NoError

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Move func to test class since that is only place it was used

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Log warning if save to cache errors

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Condense a little

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Use returned error instead of capturing it

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Bringing back ctx and adding comment

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Log error if changing ring state fails when Leaving

Signed-off-by: Nathan Zender <github@nathanzender.com>

* If context deadline exceeded return the error

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Can't defer this otherwise we will have no data

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Comment to make it clear why this nolint was added

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Refactor method out

Since Fixture is already in testutils and it is being used in both
places pulled it out into a common helper method in the testutils
package.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* io.Copy added to global errcheck exclude

Signed-off-by: Nathan Zender <github@nathanzender.com>

* If error dont do anything else

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Cleanup unused function

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Adding tracer to global excludes

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Cleanup post rebase

Formatting and import issues that got missed when merging.

Signed-off-by: Nathan Zender <github@nathanzender.com>

* Ratelimiter returns resource exhausted error

This is necessary so that when it is used with the backoff retry it will
allow for the backoff to continue to work as expected.

Signed-off-by: Nathan Zender <github@nathanzender.com>

pkg/ring/kv/consul/client.go

@@ -100,7 +100,6 @@ func (c *Client) cas(ctx context.Context, key string, f func(in interface{}) (ou
 	var (
 		index   = uint64(0)
 		retries = 10
-		retry   = true
 	)
 	for i := 0; i < retries; i++ {
 		options := &consul.QueryOptions{
@@ -124,7 +123,7 @@ func (c *Client) cas(ctx context.Context, key string, f func(in interface{}) (ou
 			intermediate = out
 		}
 
-		intermediate, retry, err = f(intermediate)
+		intermediate, retry, err := f(intermediate)
 		if err != nil {
 			if !retry {
 				if resp, ok := httpgrpc.HTTPResponseFromError(err); ok && resp.GetCode() != 202 {

pkg/ring/kv/memberlist/memberlist_client_test.go

@@ -476,7 +476,7 @@ func runClient(t *testing.T, kv *Client, name string, ringKey string, portToConn
 			if portToConnect > 0 {
 				_, err := kv.kv.JoinMembers([]string{fmt.Sprintf("127.0.0.1:%d", portToConnect)})
 				if err != nil {
-					t.Fatalf("%s failed to join the cluster: %v", name, err)
+					t.Errorf("%s failed to join the cluster: %v", name, err)
 					return
 				}
 			}

pkg/ring/kv/memberlist/tcp_transport.go

@@ -267,7 +267,7 @@ func (t *TCPTransport) handleConnection(conn *net.TCPConn) {
 
 		expectedDigest := md5.Sum(buf)
 
-		if bytes.Compare(receivedDigest, expectedDigest[:]) != 0 {
+		if !bytes.Equal(receivedDigest, expectedDigest[:]) {
 			t.receivedPacketsErrors.Inc()
 			level.Warn(util.Logger).Log("msg", "TCPTransport: packet digest mismatch", "expected", fmt.Sprintf("%x", expectedDigest), "received", fmt.Sprintf("%x", receivedDigest))
 		}

pkg/ring/kv/metrics.go

@@ -39,14 +39,14 @@ func (m metrics) CAS(ctx context.Context, key string, f func(in interface{}) (ou
 }
 
 func (m metrics) WatchKey(ctx context.Context, key string, f func(interface{}) bool) {
-	instrument.CollectedRequest(ctx, "WatchKey", requestDuration, instrument.ErrorCode, func(ctx context.Context) error {
+	_ = instrument.CollectedRequest(ctx, "WatchKey", requestDuration, instrument.ErrorCode, func(ctx context.Context) error {
 		m.c.WatchKey(ctx, key, f)
 		return nil
 	})
 }
 
 func (m metrics) WatchPrefix(ctx context.Context, prefix string, f func(string, interface{}) bool) {
-	instrument.CollectedRequest(ctx, "WatchPrefix", requestDuration, instrument.ErrorCode, func(ctx context.Context) error {
+	_ = instrument.CollectedRequest(ctx, "WatchPrefix", requestDuration, instrument.ErrorCode, func(ctx context.Context) error {
 		m.c.WatchPrefix(ctx, prefix, f)
 		return nil
 	})

pkg/ring/lifecycler.go

@@ -202,7 +202,7 @@ func (i *Lifecycler) CheckReady(ctx context.Context) error {
 
 	// Ingester always take at least minReadyDuration to become ready to work
 	// around race conditions with ingesters exiting and updating the ring
-	if time.Now().Sub(i.startTime) < i.cfg.MinReadyDuration {
+	if time.Since(i.startTime) < i.cfg.MinReadyDuration {
 		return fmt.Errorf("waiting for %v after startup", i.cfg.MinReadyDuration)
 	}
 
@@ -417,7 +417,10 @@ loop:
 	}
 
 	// Mark ourselved as Leaving so no more samples are send to us.
-	i.changeState(context.Background(), LEAVING)
+	err := i.changeState(context.Background(), LEAVING)
+	if err != nil {
+		level.Error(util.Logger).Log("msg", "failed to set state to LEAVING", "ring", i.RingName, "err", err)
+	}
 
 	// Do the transferring / flushing on a background goroutine so we can continue
 	// to heartbeat to consul.

pkg/ring/ring.go

@@ -59,12 +59,6 @@ const (
 	Reporting // Special value for inquiring about health
 )
 
-type uint32s []uint32
-
-func (x uint32s) Len() int           { return len(x) }
-func (x uint32s) Less(i, j int) bool { return x[i] < x[j] }
-func (x uint32s) Swap(i, j int)      { x[i], x[j] = x[j], x[i] }
-
 // ErrEmptyRing is the error returned when trying to get an element when nothing has been added to hash.
 var ErrEmptyRing = errors.New("empty ring")