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 7897baf commit 6715ffc
Show file tree
Hide file tree
Showing 14 changed files with 88 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 @@ -145,7 +145,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 @@ -227,6 +227,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 attempt to reconnect later
// and will not end attempts on authorization failures.
//nolint: errcheck
func Connect(addr string, opts Opts) (conn *Connection, err error) {
conn = &Connection{
addr: addr,
Expand Down Expand Up @@ -494,7 +495,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 @@ -537,6 +538,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 @@ -566,6 +568,7 @@ func (conn *Connection) unlockShards() {
}
}

//nolint: errcheck
func (conn *Connection) pinger() {
to := conn.opts.Timeout
if to == 0 {
Expand Down Expand Up @@ -687,7 +690,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 @@ -795,9 +798,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 @@ -824,7 +827,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
5 changes: 3 additions & 2 deletions 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 Down Expand Up @@ -85,6 +85,7 @@ func ExampleConnection_SelectTyped() {
// response is [{{} 1111 hello world}]
}

//nolint
func Example() {
spaceNo := uint32(512)
indexNo := uint32(0)
Expand All @@ -99,7 +100,7 @@ func Example() {
}
client, err := tarantool.Connect(server, opts)
if err != nil {
fmt.Errorf("Failed to connect: %s", err.Error())
fmt.Printf("Failed to connect: %s", err.Error())
return
}

Expand Down
1 change: 1 addition & 0 deletions queue/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/tarantool/go-tarantool/queue"
)

//nolint
func ExampleConnection_Queue() {
cfg := queue.Cfg{
Temporary: false,
Expand Down
1 change: 1 addition & 0 deletions queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ type queueData struct {
result interface{}
}

//nolint
func (qd *queueData) DecodeMsgpack(d *msgpack.Decoder) error {
var err error
var l int
Expand Down
12 changes: 10 additions & 2 deletions queue/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,15 +794,20 @@ 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
}
}()

Expand All @@ -817,6 +822,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 @@ -219,6 +223,7 @@ func (conn *Connection) SelectAsync(space, index interface{}, offset, limit, ite
if err != nil {
return future.fail(conn, err)
}
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(6)
future.fillIterator(enc, offset, limit, iterator)
Expand All @@ -234,6 +239,7 @@ func (conn *Connection) InsertAsync(space interface{}, tuple interface{}) *Futur
if err != nil {
return future.fail(conn, err)
}
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(2)
return future.fillInsert(enc, spaceNo, tuple)
Expand All @@ -248,6 +254,7 @@ func (conn *Connection) ReplaceAsync(space interface{}, tuple interface{}) *Futu
if err != nil {
return future.fail(conn, err)
}
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(2)
return future.fillInsert(enc, spaceNo, tuple)
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 @@ -276,6 +284,7 @@ func (conn *Connection) UpdateAsync(space, index interface{}, key, ops interface
if err != nil {
return future.fail(conn, err)
}
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(4)
if err := future.fillSearch(enc, spaceNo, indexNo, key); err != nil {
Expand All @@ -294,6 +303,7 @@ func (conn *Connection) UpsertAsync(space interface{}, tuple interface{}, ops in
if err != nil {
return future.fail(conn, err)
}
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(3)
enc.EncodeUint64(KeySpaceNo)
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 @@ -325,6 +336,7 @@ func (conn *Connection) CallAsync(functionName string, args interface{}) *Future
// (though, keep in mind, result is always array)
func (conn *Connection) Call17Async(functionName string, args interface{}) *Future {
future := conn.newFuture(Call17Request)
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(2)
enc.EncodeUint64(KeyFunctionName)
Expand All @@ -337,6 +349,7 @@ func (conn *Connection) Call17Async(functionName string, args interface{}) *Futu
// EvalAsync sends a lua expression for evaluation and returns Future.
func (conn *Connection) EvalAsync(expr string, args interface{}) *Future {
future := conn.newFuture(EvalRequest)
//nolint: errcheck
return future.send(conn, func(enc *msgpack.Encoder) error {
enc.EncodeMapLen(2)
enc.EncodeUint64(KeyExpression)
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
3 changes: 3 additions & 0 deletions response.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ type Response struct {
buf smallBuf
}

//nolint: unused
func (resp *Response) fill(b []byte) {
resp.buf.b = b
}

//nolint: errcheck
func (resp *Response) smallInt(d *msgpack.Decoder) (i int, err error) {
b, err := resp.buf.ReadByte()
if err != nil {
Expand All @@ -31,6 +33,7 @@ func (resp *Response) smallInt(d *msgpack.Decoder) (i int, err error) {
return d.DecodeInt()
}

//nolint: errcheck
func (resp *Response) decodeHeader(d *msgpack.Decoder) (err error) {
var l int
d.Reset(&resp.buf)
Expand Down
1 change: 1 addition & 0 deletions schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type IndexField struct {
Type string
}

//nolint: varcheck,deadcode
const (
maxSchemas = 10000
spaceSpId = 280
Expand Down
Loading

0 comments on commit 6715ffc

Please sign in to comment.