diff --git a/.golangci.yaml b/.golangci.yaml index e41addf..61d23cb 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -21,10 +21,22 @@ linters: - goconst - goimports - gosec + - gosimple - govet - ineffassign - revive - typecheck - exportloopref - prealloc - - wsl \ No newline at end of file + # - wls # excluded from linters list because produces too many noise + - staticcheck + - unused + - contextcheck + - durationcheck + - errname + - exhaustive + - gocritic + - gofmt + - nilerr + - nilnil + - usestdlibvars diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a32e84..144d917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ BUG FIXES: * RouterCallImpl: fix decoding the response from vshard.storage.call * RouterCallImpl: do not return nil error when StorageCallAssertError has happened +* BucketStat: always returns non-nil err, fixed +* DiscoveryAllBuckets returns nil even if errGr.Wait() returns err, fixed +* DiscoveryHandleBuckets: misusage of atomics, fixed FEATURES: @@ -13,6 +16,7 @@ FEATURES: REFACTOR: * Refactored docs (add QuickStart doc) and that library base on vhsard router +* Several linters are enabled because they are usefull diff --git a/api.go b/api.go index d7b1666..a2930cf 100644 --- a/api.go +++ b/api.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" @@ -64,6 +64,10 @@ type CallOpts struct { Timeout time.Duration } +// revive warns us: time-naming: var CallTimeoutMin is of type time.Duration; don't use unit-specific suffix "Min". +// But the original lua vshard implementation uses this naming, so we use it too. +// +//nolint:revive const CallTimeoutMin = time.Second / 2 // RouterCallImpl Perform shard operation function will restart operation diff --git a/discovery.go b/discovery.go index cf1ddd9..5e19163 100644 --- a/discovery.go +++ b/discovery.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" @@ -127,13 +127,11 @@ func (r *Router) DiscoveryHandleBuckets(ctx context.Context, rs *Replicaset, buc count++ if oldRs != nil { - bc := oldRs.bucketCount - if _, exists := affected[oldRs]; !exists { - affected[oldRs] = int(bc.Load()) + affected[oldRs] = int(oldRs.bucketCount.Load()) } - oldRs.bucketCount.Store(bc.Load() - 1) + oldRs.bucketCount.Add(-1) } else { // router.known_bucket_count = router.known_bucket_count + 1 r.knownBucketCount.Add(1) @@ -143,13 +141,13 @@ func (r *Router) DiscoveryHandleBuckets(ctx context.Context, rs *Replicaset, buc } if count != rs.bucketCount.Load() { - r.cfg.Logger.Info(ctx, fmt.Sprintf("Updated %s buckets: was %d, became %d", rs.info.Name, rs.bucketCount, count)) + r.cfg.Logger.Info(ctx, fmt.Sprintf("Updated %s buckets: was %d, became %d", rs.info.Name, rs.bucketCount.Load(), count)) } rs.bucketCount.Store(count) for rs, oldBucketCount := range affected { - r.log().Info(ctx, fmt.Sprintf("Affected buckets of %s: was %d, became %d", rs.info.Name, oldBucketCount, rs.bucketCount)) + r.log().Info(ctx, fmt.Sprintf("Affected buckets of %s: was %d, became %d", rs.info.Name, oldBucketCount, rs.bucketCount.Load())) } } @@ -218,7 +216,7 @@ func (r *Router) DiscoveryAllBuckets(ctx context.Context) error { err := errGr.Wait() if err != nil { - return nil + return fmt.Errorf("errGr.Wait() err: %w", err) } r.log().Info(ctx, fmt.Sprintf("discovery done since: %s", time.Since(t))) diff --git a/discovery_test.go b/discovery_test.go index 08bd81c..a059832 100644 --- a/discovery_test.go +++ b/discovery_test.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "sync" diff --git a/error.go b/error.go index 38d8ba9..8a065a2 100644 --- a/error.go +++ b/error.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import "fmt" diff --git a/providers.go b/providers.go index 740ced3..cc9a9a5 100644 --- a/providers.go +++ b/providers.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" diff --git a/providers/static/provider_test.go b/providers/static/provider_test.go index fcef97d..06ffe37 100644 --- a/providers/static/provider_test.go +++ b/providers/static/provider_test.go @@ -17,7 +17,7 @@ func TestNewProvider(t *testing.T) { {nil}, {make(map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo)}, {map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo{ - vshardrouter.ReplicasetInfo{}: { + {}: { vshardrouter.InstanceInfo{}, vshardrouter.InstanceInfo{}, }, @@ -51,7 +51,7 @@ func TestProvider_Validate(t *testing.T) { { Name: "empty name", Source: map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo{ - vshardrouter.ReplicasetInfo{}: { + {}: { vshardrouter.InstanceInfo{}, vshardrouter.InstanceInfo{}, }, @@ -61,7 +61,7 @@ func TestProvider_Validate(t *testing.T) { { Name: "no uuid", Source: map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo{ - vshardrouter.ReplicasetInfo{Name: "rs_1"}: { + {Name: "rs_1"}: { vshardrouter.InstanceInfo{}, vshardrouter.InstanceInfo{}, }, @@ -71,7 +71,7 @@ func TestProvider_Validate(t *testing.T) { { Name: "valid", Source: map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo{ - vshardrouter.ReplicasetInfo{Name: "rs_1", UUID: uuid.New()}: { + {Name: "rs_1", UUID: uuid.New()}: { vshardrouter.InstanceInfo{}, vshardrouter.InstanceInfo{}, }, diff --git a/providers/viper/provider.go b/providers/viper/provider.go index 4dc3324..7ab25af 100644 --- a/providers/viper/provider.go +++ b/providers/viper/provider.go @@ -108,9 +108,7 @@ func (p *Provider) Init(c vshardrouter.TopologyController) error { return c.AddReplicasets(context.TODO(), p.rs) } -func (p *Provider) Close() { - return -} +func (p *Provider) Close() {} type ClusterInfo struct { ReplicasetUUID string `yaml:"replicaset_uuid" mapstructure:"replicaset_uuid"` diff --git a/replicaset.go b/replicaset.go index c4ac457..6a4261c 100644 --- a/replicaset.go +++ b/replicaset.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" @@ -38,36 +38,49 @@ func (rs *Replicaset) String() string { } func (rs *Replicaset) BucketStat(ctx context.Context, bucketID uint64) (BucketStatInfo, error) { - bsInfo := &BucketStatInfo{} - bsError := &BucketStatError{} + const bucketStatFnc = "vshard.storage.bucket_stat" - req := tarantool.NewCallRequest("vshard.storage.bucket_stat"). + var bsInfo BucketStatInfo + + req := tarantool.NewCallRequest(bucketStatFnc). Args([]interface{}{bucketID}). Context(ctx) future := rs.conn.Do(req, pool.RO) respData, err := future.Get() if err != nil { - return BucketStatInfo{}, err + return bsInfo, err } - var tmp interface{} // todo: fix non-panic crutch + if len(respData) < 1 { + return bsInfo, fmt.Errorf("respData len is 0 for %s", bucketStatFnc) + } if respData[0] == nil { - err := future.GetTyped(&[]interface{}{tmp, bsError}) - if err != nil { - return BucketStatInfo{}, err + + if len(respData) < 2 { + return bsInfo, fmt.Errorf("respData len < 2 when respData[0] is nil for %s", bucketStatFnc) } - } else { - // fucking key-code 1 - // todo: fix after https://github.com/tarantool/go-tarantool/issues/368 - err := mapstructure.Decode(respData[0], bsInfo) + + var tmp interface{} // todo: fix non-panic crutch + bsError := &BucketStatError{} + + err := future.GetTyped(&[]interface{}{tmp, bsError}) if err != nil { - return BucketStatInfo{}, err + return bsInfo, err } + + return bsInfo, bsError + } + + // A problem with key-code 1 + // todo: fix after https://github.com/tarantool/go-tarantool/issues/368 + err = mapstructure.Decode(respData[0], bsInfo) + if err != nil { + return bsInfo, fmt.Errorf("can't decode bsInfo: %w", err) } - return *bsInfo, bsError + return bsInfo, nil } // ReplicaCall perform function on remote storage diff --git a/replicaset_test.go b/replicaset_test.go index 35f9abf..b7d300e 100644 --- a/replicaset_test.go +++ b/replicaset_test.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "bytes" diff --git a/topology.go b/topology.go index aa15dec..a2ae9d6 100644 --- a/topology.go +++ b/topology.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" @@ -123,7 +123,7 @@ func (c *controller) AddReplicasets(ctx context.Context, replicasets map[Replica return nil } -func (c *controller) RemoveReplicaset(ctx context.Context, rsID uuid.UUID) []error { +func (c *controller) RemoveReplicaset(_ context.Context, rsID uuid.UUID) []error { r := c.r rs := r.idToReplicaset[rsID] diff --git a/topology_test.go b/topology_test.go index b0a0311..c9bc687 100644 --- a/topology_test.go +++ b/topology_test.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" diff --git a/vshard.go b/vshard.go index a3e7021..8ffc1bd 100644 --- a/vshard.go +++ b/vshard.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "context" @@ -92,7 +92,7 @@ func (ii InstanceInfo) Validate() error { func NewRouter(ctx context.Context, cfg Config) (*Router, error) { var err error - cfg, err = prepareCfg(ctx, cfg) + cfg, err = prepareCfg(cfg) if err != nil { return nil, err } @@ -184,7 +184,7 @@ func (r *Router) RouteMapClean() { } } -func prepareCfg(ctx context.Context, cfg Config) (Config, error) { +func prepareCfg(cfg Config) (Config, error) { err := validateCfg(cfg) if err != nil { return Config{}, fmt.Errorf("%v: %v", ErrInvalidConfig, err) @@ -238,8 +238,12 @@ func (r *Router) RouterBucketIDStrCRC32(shardKey string) uint64 { return BucketIDStrCRC32(shardKey, r.cfg.TotalBucketCount) } -// RouterBucketIDMPCRC32 is not supported now -func RouterBucketIDMPCRC32(total uint64, keys ...string) {} +// RouterBucketIDMPCRC32 is not implemented yet +func RouterBucketIDMPCRC32(total uint64, keys ...string) { + // todo: implement + _, _ = total, keys + panic("RouterBucketIDMPCRC32 is not implemented yet") +} func (r *Router) RouterBucketCount() uint64 { return r.cfg.TotalBucketCount diff --git a/vshard_shadow_test.go b/vshard_shadow_test.go index 5a5cdd1..659acfd 100644 --- a/vshard_shadow_test.go +++ b/vshard_shadow_test.go @@ -24,7 +24,7 @@ func TestNewRouter_InvalidReplicasetUUID(t *testing.T) { router, err := vshard_router.NewRouter(ctx, vshard_router.Config{ TopologyProvider: static.NewProvider(map[vshard_router.ReplicasetInfo][]vshard_router.InstanceInfo{ - vshard_router.ReplicasetInfo{ + { Name: "123", }: { {Addr: "first.internal:1212"}, @@ -41,7 +41,7 @@ func TestNewRouter_InstanceAddr(t *testing.T) { router, err := vshard_router.NewRouter(ctx, vshard_router.Config{ TopologyProvider: static.NewProvider(map[vshard_router.ReplicasetInfo][]vshard_router.InstanceInfo{ - vshard_router.ReplicasetInfo{ + { Name: "123", UUID: uuid.New(), }: { diff --git a/vshard_test.go b/vshard_test.go index e82eae6..3dc5911 100644 --- a/vshard_test.go +++ b/vshard_test.go @@ -1,4 +1,4 @@ -package vshard_router +package vshard_router //nolint:revive import ( "testing"