From 409426214de20fd463ac35109cbd6368c7e17504 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 13 Sep 2024 16:00:50 -0700 Subject: [PATCH 1/4] Upgrade to go lint 1.61 --- .github/workflows/pr_build_and_test.yaml | 2 +- .golangci.yaml | 3 +++ cmd/coordinator/cmd.go | 2 +- common/security/tls.go | 4 ++-- maelstrom/main.go | 2 +- maelstrom/messages.go | 2 +- oxia/async_client_impl.go | 18 +++++++++--------- oxia/internal/metrics/metrics.go | 4 ++-- oxia/internal/shard_manager.go | 6 +++--- server/assignment_dispatcher_test.go | 6 +++--- server/auth/interceptor.go | 8 ++++---- server/session_manager_test.go | 2 +- server/wal/treemap.go | 2 +- server/wal/wal_ro_segment.go | 4 ++-- 14 files changed, 34 insertions(+), 31 deletions(-) diff --git a/.github/workflows/pr_build_and_test.yaml b/.github/workflows/pr_build_and_test.yaml index a5b7a7c6..29eb2d9d 100644 --- a/.github/workflows/pr_build_and_test.yaml +++ b/.github/workflows/pr_build_and_test.yaml @@ -61,7 +61,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@0ad9a0988b3973e851ab0a07adf248ec2e100376 #v3.3.1 with: - version: v1.55.2 + version: v1.61.1 args: --timeout=10m skip-pkg-cache: true diff --git a/.golangci.yaml b/.golangci.yaml index 1e91338e..056d9c77 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -125,6 +125,9 @@ linters-settings: - name: bare-return severity: warning disabled: false + gosec: + excludes: + - G115 issues: fix: true diff --git a/cmd/coordinator/cmd.go b/cmd/coordinator/cmd.go index 298e95c5..26dea902 100644 --- a/cmd/coordinator/cmd.go +++ b/cmd/coordinator/cmd.go @@ -143,7 +143,7 @@ func exec(*cobra.Command, []string) error { return loadClusterConfig(v) } - v.OnConfigChange(func(e fsnotify.Event) { + v.OnConfigChange(func(_ fsnotify.Event) { conf.ClusterConfigChangeNotifications <- nil }) diff --git a/common/security/tls.go b/common/security/tls.go index 2d27b24e..24ecbaaf 100644 --- a/common/security/tls.go +++ b/common/security/tls.go @@ -118,7 +118,7 @@ func (tls *TLSOption) MakeClientTLSConf() (*libtls.Config, error) { tlsConf.RootCAs = certPool } - tlsConf.GetClientCertificate = func(unused *libtls.CertificateRequestInfo) (cert *libtls.Certificate, err error) { + tlsConf.GetClientCertificate = func(_ *libtls.CertificateRequestInfo) (cert *libtls.Certificate, err error) { c, err := libtls.LoadX509KeyPair(tls.CertFile, tls.KeyFile) return &c, err } @@ -130,7 +130,7 @@ func (tls *TLSOption) MakeServerTLSConf() (*libtls.Config, error) { if err != nil { return nil, err } - tlsConf.GetCertificate = func(clientHello *libtls.ClientHelloInfo) (cert *libtls.Certificate, err error) { + tlsConf.GetCertificate = func(_ *libtls.ClientHelloInfo) (cert *libtls.Certificate, err error) { c, err := libtls.LoadX509KeyPair(tls.CertFile, tls.KeyFile) return &c, err } diff --git a/maelstrom/main.go b/maelstrom/main.go index a500bb88..36535d40 100644 --- a/maelstrom/main.go +++ b/maelstrom/main.go @@ -97,7 +97,7 @@ func receiveInit(scanner *bufio.Scanner) error { init := req.(*Message[Init]) thisNode = init.Body.NodeId - allNodes = init.Body.NodesIds + allNodes = init.Body.NodesIDs slog.Info( "Received init request", diff --git a/maelstrom/messages.go b/maelstrom/messages.go index 2c90a404..9640facb 100644 --- a/maelstrom/messages.go +++ b/maelstrom/messages.go @@ -122,7 +122,7 @@ type BaseMessageBody struct { type Init struct { BaseMessageBody NodeId string `json:"node_id"` - NodesIds []string `json:"node_ids"` + NodesIDs []string `json:"node_ids"` } type Read struct { diff --git a/oxia/async_client_impl.go b/oxia/async_client_impl.go index 4d2cec1b..849f9966 100644 --- a/oxia/async_client_impl.go +++ b/oxia/async_client_impl.go @@ -184,10 +184,10 @@ func (c *clientImpl) DeleteRange(minKeyInclusive string, maxKeyExclusive string, } // If there is no partition key, we will make the request to delete-range on all the shards - shardIds := c.shardManager.GetAll() - wg := common.NewWaitGroup(len(shardIds)) + shardIDs := c.shardManager.GetAll() + wg := common.NewWaitGroup(len(shardIDs)) - for _, shardId := range shardIds { + for _, shardId := range shardIDs { // chInner := make(chan error, 1) c.writeBatchManager.Get(shardId).Add(model.DeleteRangeCall{ MinKeyInclusive: minKeyInclusive, @@ -360,10 +360,10 @@ func (c *clientImpl) List(ctx context.Context, minKeyInclusive string, maxKeyExc }() } else { // Do the list on all shards and aggregate the responses - shardIds := c.shardManager.GetAll() + shardIDs := c.shardManager.GetAll() - wg := common.NewWaitGroup(len(shardIds)) - for _, shardId := range shardIds { + wg := common.NewWaitGroup(len(shardIDs)) + for _, shardId := range shardIDs { shardIdPtr := shardId go func() { defer wg.Done() @@ -425,10 +425,10 @@ func (c *clientImpl) RangeScan(ctx context.Context, minKeyInclusive string, maxK }() } else { // Do the list on all shards and aggregate the responses - shardIds := c.shardManager.GetAll() - channels := make([]chan GetResult, len(shardIds)) + shardIDs := c.shardManager.GetAll() + channels := make([]chan GetResult, len(shardIDs)) - for i, shardId := range shardIds { + for i, shardId := range shardIDs { shardIdPtr := shardId ch := make(chan GetResult) channels[i] = ch diff --git a/oxia/internal/metrics/metrics.go b/oxia/internal/metrics/metrics.go index 02c89ed9..dba85067 100644 --- a/oxia/internal/metrics/metrics.go +++ b/oxia/internal/metrics/metrics.go @@ -110,7 +110,7 @@ func (m *Metrics) DecorateGet(get model.GetCall) model.GetCall { func (m *Metrics) WriteCallback() func(time.Time, *proto.WriteRequest, *proto.WriteResponse, error) { metricContext := m.metricContextFunc("write") - return func(executionStart time.Time, request *proto.WriteRequest, response *proto.WriteResponse, err error) { + return func(executionStart time.Time, request *proto.WriteRequest, _ *proto.WriteResponse, err error) { ctx, batchStart, _attrs := metricContext(err) m.batchTotalTime.Record(ctx, m.sinceFunc(batchStart), _attrs) m.batchExecTime.Record(ctx, m.sinceFunc(executionStart), _attrs) @@ -122,7 +122,7 @@ func (m *Metrics) WriteCallback() func(time.Time, *proto.WriteRequest, *proto.Wr func (m *Metrics) ReadCallback() func(time.Time, *proto.ReadRequest, *proto.ReadResponse, error) { metricContext := m.metricContextFunc("read") - return func(executionStart time.Time, request *proto.ReadRequest, response *proto.ReadResponse, err error) { + return func(executionStart time.Time, _ *proto.ReadRequest, response *proto.ReadResponse, err error) { ctx, batchStart, attrs := metricContext(err) m.batchTotalTime.Record(ctx, m.sinceFunc(batchStart), attrs) m.batchExecTime.Record(ctx, m.sinceFunc(executionStart), attrs) diff --git a/oxia/internal/shard_manager.go b/oxia/internal/shard_manager.go index b1ec64f1..ce833987 100644 --- a/oxia/internal/shard_manager.go +++ b/oxia/internal/shard_manager.go @@ -117,11 +117,11 @@ func (s *shardManagerImpl) GetAll() []int64 { s.RLock() defer s.RUnlock() - shardIds := make([]int64, 0, len(s.shards)) + shardIDs := make([]int64, 0, len(s.shards)) for shardId := range s.shards { - shardIds = append(shardIds, shardId) + shardIDs = append(shardIDs, shardId) } - return shardIds + return shardIDs } func (s *shardManagerImpl) Leader(shardId int64) string { diff --git a/server/assignment_dispatcher_test.go b/server/assignment_dispatcher_test.go index 49455ec1..83959acd 100644 --- a/server/assignment_dispatcher_test.go +++ b/server/assignment_dispatcher_test.go @@ -324,14 +324,14 @@ func TestShardAssignmentDispatcher_MultipleNamespaces(t *testing.T) { assert.NoError(t, dispatcher.Close()) } -func newShardAssignment(id int64, leader string, min uint32, max uint32) *proto.ShardAssignment { +func newShardAssignment(id int64, leader string, minHashInclusive uint32, maxHashInclusive uint32) *proto.ShardAssignment { return &proto.ShardAssignment{ Shard: id, Leader: leader, ShardBoundaries: &proto.ShardAssignment_Int32HashRange{ Int32HashRange: &proto.Int32HashRange{ - MinHashInclusive: min, - MaxHashInclusive: max, + MinHashInclusive: minHashInclusive, + MaxHashInclusive: maxHashInclusive, }, }, } diff --git a/server/auth/interceptor.go b/server/auth/interceptor.go index 5cdb5604..02950dcc 100644 --- a/server/auth/interceptor.go +++ b/server/auth/interceptor.go @@ -43,10 +43,10 @@ type GrpcAuthenticationDelegator struct { } func (delegator *GrpcAuthenticationDelegator) GetUnaryInterceptor() grpc.UnaryServerInterceptor { - return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { + return func(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { _, err := delegator.validate(ctx, delegator.provider) if err != nil { - return nil, status.Errorf(codes.Unauthenticated, err.Error()) + return nil, status.Error(codes.Unauthenticated, err.Error()) } // todo: set username to metadata to support authorization return handler(ctx, req) @@ -54,10 +54,10 @@ func (delegator *GrpcAuthenticationDelegator) GetUnaryInterceptor() grpc.UnarySe } func (delegator *GrpcAuthenticationDelegator) GetStreamInterceptor() grpc.StreamServerInterceptor { - return func(srv any, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { + return func(srv any, ss grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error { _, err := delegator.validate(ss.Context(), delegator.provider) if err != nil { - return status.Errorf(codes.Unauthenticated, err.Error()) + return status.Error(codes.Unauthenticated, err.Error()) } // todo: set username to metadata to support authorization return handler(srv, ss) diff --git a/server/session_manager_test.go b/server/session_manager_test.go index cab4336a..741162f6 100644 --- a/server/session_manager_test.go +++ b/server/session_manager_test.go @@ -106,7 +106,7 @@ func (m mockWriteBatch) DeleteRange(_, _ string) error { } func (m mockWriteBatch) KeyRangeScan(_, _ string) (kv.KeyIterator, error) { - return nil, nil + return nil, kv.ErrKeyNotFound } func (m mockWriteBatch) Commit() error { diff --git a/server/wal/treemap.go b/server/wal/treemap.go index d1e57e56..2b3b5c44 100644 --- a/server/wal/treemap.go +++ b/server/wal/treemap.go @@ -58,7 +58,7 @@ func (m *treeMap[K, V]) Size() int { func (m *treeMap[K, V]) Keys() []K { keys := make([]K, 0, m.tree.Size()) - m.Each(func(k K, v V) bool { + m.Each(func(k K, _ V) bool { keys = append(keys, k) return true }) diff --git a/server/wal/wal_ro_segment.go b/server/wal/wal_ro_segment.go index 59a55eb3..c4f34b1d 100644 --- a/server/wal/wal_ro_segment.go +++ b/server/wal/wal_ro_segment.go @@ -211,7 +211,7 @@ func (r *readOnlySegmentsGroup) Close() error { defer r.Unlock() var err error - r.openSegments.Each(func(id int64, segment common.RefCount[ReadOnlySegment]) bool { + r.openSegments.Each(func(_ int64, segment common.RefCount[ReadOnlySegment]) bool { err = multierr.Append(err, segment.(io.Closer).Close()) return true }) @@ -293,7 +293,7 @@ func (r *readOnlySegmentsGroup) PollHighestSegment() (common.RefCount[ReadOnlySe defer r.Unlock() if r.allSegments.Empty() { - return nil, nil + return nil, nil //nolint: nilnil } offset, _ := r.allSegments.Max() From 36b41aa0cd3f504cc81d0e8ac9436818bf2a9f75 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 13 Sep 2024 16:06:51 -0700 Subject: [PATCH 2/4] Fixed version --- .github/workflows/pr_build_and_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_build_and_test.yaml b/.github/workflows/pr_build_and_test.yaml index 29eb2d9d..fa153a3c 100644 --- a/.github/workflows/pr_build_and_test.yaml +++ b/.github/workflows/pr_build_and_test.yaml @@ -61,7 +61,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@0ad9a0988b3973e851ab0a07adf248ec2e100376 #v3.3.1 with: - version: v1.61.1 + version: v1.61.0 args: --timeout=10m skip-pkg-cache: true From 8a9ce205a00e902271e50abf9e36c15a3089a44b Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 13 Sep 2024 16:19:07 -0700 Subject: [PATCH 3/4] Fixed GH action --- .github/workflows/pr_build_and_test.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr_build_and_test.yaml b/.github/workflows/pr_build_and_test.yaml index fa153a3c..e4519309 100644 --- a/.github/workflows/pr_build_and_test.yaml +++ b/.github/workflows/pr_build_and_test.yaml @@ -59,11 +59,10 @@ jobs: run: make build - name: golangci-lint - uses: golangci/golangci-lint-action@0ad9a0988b3973e851ab0a07adf248ec2e100376 #v3.3.1 + uses: golangci/golangci-lint-action@v6.1.0 with: version: v1.61.0 args: --timeout=10m - skip-pkg-cache: true - name: Test run: make test From 2d2b772cda0cf1edb29234b0f936eb2d6754728e Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Fri, 13 Sep 2024 16:22:35 -0700 Subject: [PATCH 4/4] Fixed missing space --- server/wal/wal_ro_segment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/wal/wal_ro_segment.go b/server/wal/wal_ro_segment.go index c4f34b1d..30028fa1 100644 --- a/server/wal/wal_ro_segment.go +++ b/server/wal/wal_ro_segment.go @@ -293,7 +293,7 @@ func (r *readOnlySegmentsGroup) PollHighestSegment() (common.RefCount[ReadOnlySe defer r.Unlock() if r.allSegments.Empty() { - return nil, nil //nolint: nilnil + return nil, nil // nolint: nilnil } offset, _ := r.allSegments.Max()