Skip to content

Commit

Permalink
code health: fix all places highlighted by linter
Browse files Browse the repository at this point in the history
Removed error's suppression in check.yaml.

Suppressed the highlighting of errorcheck in all methods,
having encodeXxx inside. For now these methods are not able to
return any error cause of internal implementation of writer
interface. For future, if the implementation of writer
will be changed, and we will have to check errors, we must think
about how to say to compiler that the error check is 'unlikely'.

Fixed the use of time package API, all places with calls of
time.Now().Sub(). Now time package propose the explicit time.Until().

Replaced some calls of Errorf() with Fatalf() in tests. That change
prevents nil dereferences below in the code.

Suppressed the highlighting of all unused constants and functions.

Fixed calling of Fatalf from non-testing goroutine in queue tests.
It is not a valid way to stop test from another goroutine.

Relates to #142
  • Loading branch information
vr009 committed Apr 20, 2022
1 parent 1d06618 commit 6bd1dd1
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 35 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
args: --issues-exit-code=0 -E gofmt
args: -E gofmt
6 changes: 6 additions & 0 deletions client_tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type IntKey struct {
I int
}

//nolint: errcheck
func (k IntKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeSliceLen(1)
enc.EncodeInt(k.I)
Expand All @@ -22,6 +23,7 @@ type UintKey struct {
I uint
}

//nolint: errcheck
func (k UintKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeSliceLen(1)
enc.EncodeUint(k.I)
Expand All @@ -34,6 +36,7 @@ type StringKey struct {
S string
}

//nolint: errcheck
func (k StringKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeSliceLen(1)
enc.EncodeString(k.S)
Expand All @@ -46,6 +49,7 @@ type IntIntKey struct {
I1, I2 int
}

//nolint: errcheck
func (k IntIntKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeSliceLen(2)
enc.EncodeInt(k.I1)
Expand All @@ -60,6 +64,7 @@ type Op struct {
Arg interface{}
}

//nolint: errcheck
func (o Op) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeSliceLen(3)
enc.EncodeString(o.Op)
Expand All @@ -75,6 +80,7 @@ type OpSplice struct {
Replace string
}

//nolint: errcheck
func (o OpSplice) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeSliceLen(5)
enc.EncodeString(o.Op)
Expand Down
15 changes: 9 additions & 6 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ type connShard struct {
bufmut sync.Mutex
buf smallWBuf
enc *msgpack.Encoder
_pad [16]uint64
_pad [16]uint64 //nolint: structcheck,unused
}

// Greeting is a message sent by Tarantool on connect.
Expand Down Expand Up @@ -228,6 +228,7 @@ type Opts struct {
// - If opts.Reconnect is non-zero, then error will be returned only if authorization
// fails. But if Tarantool is not reachable, then it will make an attempt to reconnect later
// and will not finish to make attempts on authorization failures.
//nolint: errcheck
func Connect(addr string, opts Opts) (conn *Connection, err error) {
conn = &Connection{
addr: addr,
Expand Down Expand Up @@ -495,7 +496,7 @@ func (conn *Connection) createConnection(reconnect bool) (err error) {
conn.notify(ReconnectFailed)
reconnects++
conn.mutex.Unlock()
time.Sleep(now.Add(conn.opts.Reconnect).Sub(time.Now()))
time.Sleep(time.Until(now.Add(conn.opts.Reconnect)))
conn.mutex.Lock()
}
if conn.state == connClosed {
Expand Down Expand Up @@ -538,6 +539,7 @@ func (conn *Connection) closeConnection(neterr error, forever bool) (err error)
return
}

//nolint: errcheck
func (conn *Connection) reconnect(neterr error, c net.Conn) {
conn.mutex.Lock()
defer conn.mutex.Unlock()
Expand Down Expand Up @@ -567,6 +569,7 @@ func (conn *Connection) unlockShards() {
}
}

//nolint: errcheck
func (conn *Connection) pinger() {
to := conn.opts.Timeout
if to == 0 {
Expand Down Expand Up @@ -688,7 +691,7 @@ func (conn *Connection) newFuture(requestCode int32) (fut *Future) {
*pair.last = fut
pair.last = &fut.next
if conn.opts.Timeout > 0 {
fut.timeout = time.Now().Sub(epoch) + conn.opts.Timeout
fut.timeout = time.Until(epoch) + conn.opts.Timeout
}
shard.rmut.Unlock()
if conn.rlimit != nil && conn.opts.RLimitAction == RLimitWait {
Expand Down Expand Up @@ -796,9 +799,9 @@ func (conn *Connection) timeouts() {
return
case <-t.C:
}
minNext := time.Now().Sub(epoch) + timeout
minNext := time.Until(epoch) + timeout
for i := range conn.shard {
nowepoch = time.Now().Sub(epoch)
nowepoch = time.Until(epoch)
shard := &conn.shard[i]
for pos := range shard.requests {
shard.rmut.Lock()
Expand All @@ -825,7 +828,7 @@ func (conn *Connection) timeouts() {
shard.rmut.Unlock()
}
}
nowepoch = time.Now().Sub(epoch)
nowepoch = time.Until(epoch)
if nowepoch+time.Microsecond < minNext {
t.Reset(minNext - nowepoch)
} else {
Expand Down
2 changes: 2 additions & 0 deletions deadline_io.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type DeadlineIO struct {
c net.Conn
}

//nolint: errcheck
func (d *DeadlineIO) Write(b []byte) (n int, err error) {
if d.to > 0 {
d.c.SetWriteDeadline(time.Now().Add(d.to))
Expand All @@ -18,6 +19,7 @@ func (d *DeadlineIO) Write(b []byte) (n int, err error) {
return
}

//nolint: errcheck
func (d *DeadlineIO) Read(b []byte) (n int, err error) {
if d.to > 0 {
d.c.SetReadDeadline(time.Now().Add(d.to))
Expand Down
4 changes: 3 additions & 1 deletion example_custom_unpacking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ type Tuple2 struct {

// Same effect in a "magic" way, but slower.
type Tuple3 struct {
_msgpack struct{} `msgpack:",asArray"`
_msgpack struct{} `msgpack:",asArray"` //nolint: structcheck,unused

Cid uint
Orig string
Members []Member
}

//nolint: errcheck
func (c *Tuple2) EncodeMsgpack(e *msgpack.Encoder) error {
if err := e.EncodeSliceLen(3); err != nil {
return err
Expand All @@ -37,6 +38,7 @@ func (c *Tuple2) EncodeMsgpack(e *msgpack.Encoder) error {
return nil
}

//nolint: errcheck
func (c *Tuple2) DecodeMsgpack(d *msgpack.Decoder) error {
var err error
var l int
Expand Down
8 changes: 7 additions & 1 deletion example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
type Tuple struct {
// Instruct msgpack to pack this struct as array, so no custom packer
// is needed.
_msgpack struct{} `msgpack:",asArray"`
_msgpack struct{} `msgpack:",asArray"` //nolint: structcheck,unused
Id uint
Msg string
Name string
Expand All @@ -24,6 +24,7 @@ func example_connect() *tarantool.Connection {
return conn
}

//nolint: errcheck
func ExampleConnection_Select() {
conn := example_connect()
defer conn.Close()
Expand Down Expand Up @@ -70,6 +71,7 @@ func ExampleConnection_SelectTyped() {
// response is [{{} 1111 hello world}]
}

//nolint
func ExampleConnection_SelectAsync() {
conn := example_connect()
defer conn.Close()
Expand Down Expand Up @@ -118,6 +120,7 @@ func ExampleConnection_Ping() {
// Ping Error <nil>
}

//nolint: errcheck
func ExampleConnection_Insert() {
conn := example_connect()
defer conn.Close()
Expand Down Expand Up @@ -151,6 +154,7 @@ func ExampleConnection_Insert() {

}

//nolint: errcheck
func ExampleConnection_Delete() {
conn := example_connect()
defer conn.Close()
Expand Down Expand Up @@ -184,6 +188,7 @@ func ExampleConnection_Delete() {
// Data [[36 test one]]
}

//nolint: errcheck
func ExampleConnection_Replace() {
conn := example_connect()
defer conn.Close()
Expand Down Expand Up @@ -233,6 +238,7 @@ func ExampleConnection_Replace() {
// Data [[13 test twelve]]
}

//nolint: errcheck
func ExampleConnection_Update() {
conn := example_connect()
defer conn.Close()
Expand Down
1 change: 1 addition & 0 deletions queue/example_msgpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func (c *dummyData) EncodeMsgpack(e *msgpack.Encoder) error {
// - If you use the connection timeout and call Take, we return an error if we
// cannot take the task out of the queue within the time corresponding to the
// connection timeout.
//nolint: errcheck,ineffassign
func Example_simpleQueueCustomMsgPack() {
opts := tarantool.Opts{
Reconnect: time.Second,
Expand Down
1 change: 1 addition & 0 deletions queue/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/tarantool/go-tarantool/queue"
)

//nolint
// Example demonstrates an operations like Put and Take with queue.
func Example_simpleQueue() {
cfg := queue.Cfg{
Expand Down
1 change: 1 addition & 0 deletions queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ type queueData struct {
result interface{}
}

//nolint
func (qd *queueData) DecodeMsgpack(d *msgpack.Decoder) error {
var err error
var l int
Expand Down
13 changes: 11 additions & 2 deletions queue/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,16 +794,22 @@ func TestUtube_Put(t *testing.T) {
t.Fatalf("Failed put task to queue: %s", err.Error())
}

errChan := make(chan struct{})
go func() {
t1, err := q.TakeTimeout(2 * time.Second)
if err != nil {
t.Fatalf("Failed to take task from utube: %s", err.Error())
t.Errorf("Failed to take task from utube: %s", err.Error())
errChan <- struct{}{}
return
}

time.Sleep(2 * time.Second)
if err := t1.Ack(); err != nil {
t.Fatalf("Failed to ack task: %s", err.Error())
t.Errorf("Failed to ack task: %s", err.Error())
errChan <- struct{}{}
return
}
close(errChan)
}()

time.Sleep(100 * time.Millisecond)
Expand All @@ -817,6 +823,9 @@ func TestUtube_Put(t *testing.T) {
t.Fatalf("Failed to ack task: %s", err.Error())
}
end := time.Now()
if _, ok := <-errChan; ok {
t.Fatalf("One of take failed")
}
if math.Abs(float64(end.Sub(start)-2*time.Second)) > float64(200*time.Millisecond) {
t.Fatalf("Blocking time is less than expected: actual = %.2fs, expected = 1s", end.Sub(start).Seconds())
}
Expand Down
14 changes: 14 additions & 0 deletions request.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ type Future struct {
}

// Ping sends empty request to Tarantool to check connection.
//nolint: errcheck
func (conn *Connection) Ping() (resp *Response, err error) {
future := conn.newFuture(PingRequest)
return future.send(conn, func(enc *msgpack.Encoder) error { enc.EncodeMapLen(0); return nil }).Get()
}

//nolint: errcheck
func (req *Future) fillSearch(enc *msgpack.Encoder, spaceNo, indexNo uint32, key interface{}) error {
enc.EncodeUint64(KeySpaceNo)
enc.EncodeUint64(uint64(spaceNo))
Expand All @@ -33,6 +35,7 @@ func (req *Future) fillSearch(enc *msgpack.Encoder, spaceNo, indexNo uint32, key
return enc.Encode(key)
}

//nolint: errcheck
func (req *Future) fillIterator(enc *msgpack.Encoder, offset, limit, iterator uint32) {
enc.EncodeUint64(KeyIterator)
enc.EncodeUint64(uint64(iterator))
Expand All @@ -42,6 +45,7 @@ func (req *Future) fillIterator(enc *msgpack.Encoder, offset, limit, iterator ui
enc.EncodeUint64(uint64(limit))
}

//nolint: errcheck
func (req *Future) fillInsert(enc *msgpack.Encoder, spaceNo uint32, tuple interface{}) error {
enc.EncodeUint64(KeySpaceNo)
enc.EncodeUint64(uint64(spaceNo))
Expand Down Expand Up @@ -213,6 +217,7 @@ func (conn *Connection) EvalTyped(expr string, args interface{}, result interfac
}

// SelectAsync sends select request to Tarantool and returns Future.
//nolint: errcheck
func (conn *Connection) SelectAsync(space, index interface{}, offset, limit, iterator uint32, key interface{}) *Future {
future := conn.newFuture(SelectRequest)
spaceNo, indexNo, err := conn.Schema.resolveSpaceIndex(space, index)
Expand All @@ -228,6 +233,7 @@ func (conn *Connection) SelectAsync(space, index interface{}, offset, limit, ite

// InsertAsync sends insert action to Tarantool and returns Future.
// Tarantool will reject Insert when tuple with same primary key exists.
//nolint: errcheck
func (conn *Connection) InsertAsync(space interface{}, tuple interface{}) *Future {
future := conn.newFuture(InsertRequest)
spaceNo, _, err := conn.Schema.resolveSpaceIndex(space, nil)
Expand All @@ -242,6 +248,7 @@ func (conn *Connection) InsertAsync(space interface{}, tuple interface{}) *Futur

// ReplaceAsync sends "insert or replace" action to Tarantool and returns Future.
// If tuple with same primary key exists, it will be replaced.
//nolint: errcheck
func (conn *Connection) ReplaceAsync(space interface{}, tuple interface{}) *Future {
future := conn.newFuture(ReplaceRequest)
spaceNo, _, err := conn.Schema.resolveSpaceIndex(space, nil)
Expand All @@ -256,6 +263,7 @@ func (conn *Connection) ReplaceAsync(space interface{}, tuple interface{}) *Futu

// DeleteAsync sends deletion action to Tarantool and returns Future.
// Future's result will contain array with deleted tuple.
//nolint: errcheck
func (conn *Connection) DeleteAsync(space, index interface{}, key interface{}) *Future {
future := conn.newFuture(DeleteRequest)
spaceNo, indexNo, err := conn.Schema.resolveSpaceIndex(space, index)
Expand All @@ -270,6 +278,7 @@ func (conn *Connection) DeleteAsync(space, index interface{}, key interface{}) *

// Update sends deletion of a tuple by key and returns Future.
// Future's result will contain array with updated tuple.
//nolint: errcheck
func (conn *Connection) UpdateAsync(space, index interface{}, key, ops interface{}) *Future {
future := conn.newFuture(UpdateRequest)
spaceNo, indexNo, err := conn.Schema.resolveSpaceIndex(space, index)
Expand All @@ -288,6 +297,7 @@ func (conn *Connection) UpdateAsync(space, index interface{}, key, ops interface

// UpsertAsync sends "update or insert" action to Tarantool and returns Future.
// Future's sesult will not contain any tuple.
//nolint: errcheck
func (conn *Connection) UpsertAsync(space interface{}, tuple interface{}, ops interface{}) *Future {
future := conn.newFuture(UpsertRequest)
spaceNo, _, err := conn.Schema.resolveSpaceIndex(space, nil)
Expand All @@ -309,6 +319,7 @@ func (conn *Connection) UpsertAsync(space interface{}, tuple interface{}, ops in

// CallAsync sends a call to registered Tarantool function and returns Future.
// It uses request code for Tarantool 1.6, so future's result is always array of arrays
//nolint: errcheck
func (conn *Connection) CallAsync(functionName string, args interface{}) *Future {
future := conn.newFuture(CallRequest)
return future.send(conn, func(enc *msgpack.Encoder) error {
Expand All @@ -323,6 +334,7 @@ func (conn *Connection) CallAsync(functionName string, args interface{}) *Future
// Call17Async sends a call to registered Tarantool function and returns Future.
// It uses request code for Tarantool 1.7, so future's result will not be converted
// (though, keep in mind, result is always array)
//nolint: errcheck
func (conn *Connection) Call17Async(functionName string, args interface{}) *Future {
future := conn.newFuture(Call17Request)
return future.send(conn, func(enc *msgpack.Encoder) error {
Expand All @@ -335,6 +347,7 @@ func (conn *Connection) Call17Async(functionName string, args interface{}) *Futu
}

// EvalAsync sends a Lua expression for evaluation and returns Future.
//nolint: errcheck
func (conn *Connection) EvalAsync(expr string, args interface{}) *Future {
future := conn.newFuture(EvalRequest)
return future.send(conn, func(enc *msgpack.Encoder) error {
Expand All @@ -350,6 +363,7 @@ func (conn *Connection) EvalAsync(expr string, args interface{}) *Future {
// private
//

//nolint: errcheck
func (fut *Future) pack(h *smallWBuf, enc *msgpack.Encoder, body func(*msgpack.Encoder) error) (err error) {
rid := fut.requestId
hl := h.Len()
Expand Down
Loading

0 comments on commit 6bd1dd1

Please sign in to comment.