Merge branch 'master' into log_down_peer

.github/workflows/pd-tests.yaml

@@ -72,7 +72,7 @@ jobs:
           TOTAL_JOBS: ${{needs.chunks.outputs.job-total}}
         run: for i in $(seq 1 $TOTAL_JOBS); do cat covprofile_$i >> covprofile; done
       - name: Send coverage
-        uses: codecov/codecov-action@v1
+        uses: codecov/codecov-action@v4.2.0
         with:
           token: ${{ secrets.CODECOV }}
           file: ./covprofile

.golangci.yml

@@ -13,6 +13,7 @@ linters:
     - bodyclose
     - testifylint
     - gofmt
+    - revive
   disable:
     - errcheck
 linters-settings:
@@ -52,3 +53,149 @@ linters-settings:
     rewrite-rules:
       - pattern: "interface{}"
         replacement: "any"
+  revive:
+    ignore-generated-header: false
+    severity: error
+    confidence: 0.8
+    rules:
+      - name: atomic
+        severity: warning
+        exclude: [""]
+        disabled: false
+      - name: blank-imports
+        severity: warning
+        exclude: [""]
+        disabled: false
+      - name: confusing-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: confusing-results
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: context-as-argument
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - allowTypesBefore: "*testing.T,*github.com/user/repo/testing.Harness"
+      - name: datarace
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: defer
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - ["call-chain", "loop"]
+      - name: dot-imports
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: duplicated-imports
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: empty-block
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: empty-lines
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: error-return
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: error-strings
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: error-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: exported
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - "checkPrivateReceivers"
+          - "sayRepetitiveInsteadOfStutters"
+      - name: identical-branches
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: if-return
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: modifies-parameter
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: optimize-operands-order
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: package-comments
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: range
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: range-val-in-closure
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: range-val-address
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: receiver-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: indent-error-flow
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: superfluous-else
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: unnecessary-stmt
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: unreachable-code
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: unused-parameter
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - allowRegex: "^_"
+      - name: unused-receiver
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: useless-break
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: var-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: waitgroup-by-value
+        severity: warning
+        disabled: false
+        exclude: [""]

Makefile

@@ -80,7 +80,7 @@ BUILD_BIN_PATH := $(ROOT_PATH)/bin
 
 build: pd-server pd-ctl pd-recover
 
-tools: pd-tso-bench pd-heartbeat-bench regions-dump stores-dump pd-api-bench
+tools: pd-tso-bench pd-heartbeat-bench regions-dump stores-dump pd-api-bench pd-ut
 
 PD_SERVER_DEP :=
 ifeq ($(SWAGGER), 1)
@@ -108,7 +108,6 @@ pd-server-basic:
 .PHONY: pre-build build tools pd-server pd-server-basic
 
 # Tools
-
 pd-ctl:
 	cd tools && GOEXPERIMENT=$(BUILD_GOEXPERIMENT) CGO_ENABLED=$(BUILD_TOOL_CGO_ENABLED) go build -gcflags '$(GCFLAGS)' -ldflags '$(LDFLAGS)' -o $(BUILD_BIN_PATH)/pd-ctl pd-ctl/main.go
 pd-tso-bench:
@@ -127,8 +126,12 @@ regions-dump:
 	cd tools && CGO_ENABLED=0 go build -gcflags '$(GCFLAGS)' -ldflags '$(LDFLAGS)' -o $(BUILD_BIN_PATH)/regions-dump regions-dump/main.go
 stores-dump:
 	cd tools && CGO_ENABLED=0 go build -gcflags '$(GCFLAGS)' -ldflags '$(LDFLAGS)' -o $(BUILD_BIN_PATH)/stores-dump stores-dump/main.go
+pd-ut: pd-xprog
+	cd tools && GOEXPERIMENT=$(BUILD_GOEXPERIMENT) CGO_ENABLED=$(BUILD_TOOL_CGO_ENABLED) go build -gcflags '$(GCFLAGS)' -ldflags '$(LDFLAGS)' -o $(BUILD_BIN_PATH)/pd-ut pd-ut/ut.go
+pd-xprog:
+	cd tools && GOEXPERIMENT=$(BUILD_GOEXPERIMENT) CGO_ENABLED=$(BUILD_TOOL_CGO_ENABLED) go build -tags xprog -gcflags '$(GCFLAGS)' -ldflags '$(LDFLAGS)' -o $(BUILD_BIN_PATH)/xprog pd-ut/xprog.go
 
-.PHONY: pd-ctl pd-tso-bench pd-recover pd-analysis pd-heartbeat-bench simulator regions-dump stores-dump pd-api-bench
+.PHONY: pd-ctl pd-tso-bench pd-recover pd-analysis pd-heartbeat-bench simulator regions-dump stores-dump pd-api-bench pd-ut
 
 #### Docker image ####
 
@@ -181,9 +184,6 @@ static: install-tools pre-build
 	@ gofmt -s -l -d $(PACKAGE_DIRECTORIES) 2>&1 | awk '{ print } END { if (NR > 0) { exit 1 } }'
 	@ echo "golangci-lint ..."
 	@ golangci-lint run --verbose $(PACKAGE_DIRECTORIES) --allow-parallel-runners
-	@ echo "revive ..."
-	@ revive -formatter friendly -config revive.toml $(PACKAGES)
-
 	@ for mod in $(SUBMODULES); do cd $$mod && $(MAKE) static && cd $(ROOT_PATH) > /dev/null; done
 
 # Because CI downloads the dashboard code and runs gofmt, we can't add this check into static now.
@@ -225,6 +225,12 @@ failpoint-disable: install-tools
 
 #### Test ####
 
+ut: pd-ut
+	@$(FAILPOINT_ENABLE)
+	./bin/pd-ut run --race
+	@$(CLEAN_UT_BINARY)
+	@$(FAILPOINT_DISABLE)
+
 PACKAGE_DIRECTORIES := $(subst $(PD_PKG)/,,$(PACKAGES))
 TEST_PKGS := $(filter $(shell find . -iname "*_test.go" -exec dirname {} \; | \
                      sort -u | sed -e "s/^\./github.com\/tikv\/pd/"),$(PACKAGES))
@@ -303,13 +309,16 @@ split:
 
 clean: failpoint-disable clean-test clean-build
 
+CLEAN_UT_BINARY := find . -name '*.test.bin'| xargs rm -f
+
 clean-test:
 	# Cleaning test tmp...
 	rm -rf /tmp/test_pd*
 	rm -rf /tmp/pd-tests*
 	rm -rf /tmp/test_etcd*
 	rm -f $(REAL_CLUSTER_TEST_PATH)/playground.log
 	go clean -testcache
+	@$(CLEAN_UT_BINARY)
 
 clean-build:
 	# Cleaning building files...

client/Makefile

@@ -13,18 +13,18 @@
 # limitations under the License.
 
 ROOT_PATH := $(shell pwd)/..
-GO_TOOLS_BIN_PATH := $(shell pwd)/../.tools/bin
+GO_TOOLS_BIN_PATH := $(ROOT_PATH)/.tools/bin
 PATH := $(GO_TOOLS_BIN_PATH):$(PATH)
 SHELL := env PATH='$(PATH)' GOBIN='$(GO_TOOLS_BIN_PATH)' $(shell which bash)
 
 default: static tidy test
 
 test: failpoint-enable
-	CGO_ENABLE=1 go test ./... -v -tags deadlock -race -cover || { $(MAKE) failpoint-disable && exit 1; }
+	CGO_ENABLED=1 go test ./... -v -tags deadlock -race -cover || { $(MAKE) failpoint-disable && exit 1; }
 	$(MAKE) failpoint-disable
 
 basic-test: failpoint-enable
-	CGO_ENABLE=1 go test ./... || { $(MAKE) failpoint-disable && exit 1; }
+	CGO_ENABLED=1 go test ./... || { $(MAKE) failpoint-disable && exit 1; }
 	$(MAKE) failpoint-disable
 
 ci-test-job:
@@ -45,8 +45,6 @@ static: install-tools
 	@ gofmt -s -l -d . 2>&1 | awk '{ print } END { if (NR > 0) { exit 1 } }'
 	@ echo "golangci-lint ..."
 	@ golangci-lint run -c ../.golangci.yml --verbose ./... --allow-parallel-runners
-	@ echo "revive ..."
-	@ revive -formatter friendly -config ../revive.toml ./...
 
 tidy:
 	@ go mod tidy

client/client.go

@@ -69,16 +69,10 @@ type GlobalConfigItem struct {
 	PayLoad   []byte
 }
 
-// Client is a PD (Placement Driver) RPC client.
-// It should not be used after calling Close().
-type Client interface {
-	// GetClusterID gets the cluster ID from PD.
-	GetClusterID(ctx context.Context) uint64
+// RPCClient is a PD (Placement Driver) RPC and related mcs client which can only call RPC.
+type RPCClient interface {
 	// GetAllMembers gets the members Info from PD
 	GetAllMembers(ctx context.Context) ([]*pdpb.Member, error)
-	// GetLeaderURL returns current leader's URL. It returns "" before
-	// syncing leader from server.
-	GetLeaderURL() string
 	// GetRegion gets a region and its leader Peer from PD by key.
 	// The region may expire after split. Caller is responsible for caching and
 	// taking care of region change.
@@ -133,17 +127,12 @@ type Client interface {
 	StoreGlobalConfig(ctx context.Context, configPath string, items []GlobalConfigItem) error
 	// WatchGlobalConfig returns a stream with all global config and updates
 	WatchGlobalConfig(ctx context.Context, configPath string, revision int64) (chan []GlobalConfigItem, error)
-	// UpdateOption updates the client option.
-	UpdateOption(option DynamicOption, value any) error
 
 	// GetExternalTimestamp returns external timestamp
 	GetExternalTimestamp(ctx context.Context) (uint64, error)
 	// SetExternalTimestamp sets external timestamp
 	SetExternalTimestamp(ctx context.Context, timestamp uint64) error
 
-	// GetServiceDiscovery returns ServiceDiscovery
-	GetServiceDiscovery() ServiceDiscovery
-
 	// TSOClient is the TSO client.
 	TSOClient
 	// MetaStorageClient is the meta storage client.
@@ -154,6 +143,24 @@ type Client interface {
 	GCClient
 	// ResourceManagerClient manages resource group metadata and token assignment.
 	ResourceManagerClient
+}
+
+// Client is a PD (Placement Driver) RPC client.
+// It should not be used after calling Close().
+type Client interface {
+	RPCClient
+
+	// GetClusterID gets the cluster ID from PD.
+	GetClusterID(ctx context.Context) uint64
+	// GetLeaderURL returns current leader's URL. It returns "" before
+	// syncing leader from server.
+	GetLeaderURL() string
+	// GetServiceDiscovery returns ServiceDiscovery
+	GetServiceDiscovery() ServiceDiscovery
+
+	// UpdateOption updates the client option.
+	UpdateOption(option DynamicOption, value any) error
+
 	// Close closes the client.
 	Close()
 }
@@ -431,12 +438,12 @@ func NewAPIContextV1() APIContext {
 }
 
 // GetAPIVersion returns the API version.
-func (apiCtx *apiContextV1) GetAPIVersion() (version APIVersion) {
+func (*apiContextV1) GetAPIVersion() (version APIVersion) {
 	return V1
 }
 
 // GetKeyspaceName returns the keyspace name.
-func (apiCtx *apiContextV1) GetKeyspaceName() (keyspaceName string) {
+func (*apiContextV1) GetKeyspaceName() (keyspaceName string) {
 	return ""
 }
 
@@ -453,7 +460,7 @@ func NewAPIContextV2(keyspaceName string) APIContext {
 }
 
 // GetAPIVersion returns the API version.
-func (apiCtx *apiContextV2) GetAPIVersion() (version APIVersion) {
+func (*apiContextV2) GetAPIVersion() (version APIVersion) {
 	return V2
 }
 
@@ -912,7 +919,7 @@ func handleRegionResponse(res *pdpb.GetRegionResponse) *Region {
 	return r
 }
 
-func (c *client) GetRegionFromMember(ctx context.Context, key []byte, memberURLs []string, opts ...GetRegionOption) (*Region, error) {
+func (c *client) GetRegionFromMember(ctx context.Context, key []byte, memberURLs []string, _ ...GetRegionOption) (*Region, error) {
 	if span := opentracing.SpanFromContext(ctx); span != nil && span.Tracer() != nil {
 		span = span.Tracer().StartSpan("pdclient.GetRegionFromMember", opentracing.ChildOf(span.Context()))
 		defer span.Finish()

client/keyspace_client.go

@@ -128,7 +128,7 @@ func (c *client) UpdateKeyspaceState(ctx context.Context, id uint32, state keysp
 // It returns a stream of slices of keyspace metadata.
 // The first message in stream contains all current keyspaceMeta,
 // all subsequent messages contains new put events for all keyspaces.
-func (c *client) WatchKeyspaces(ctx context.Context) (chan []*keyspacepb.KeyspaceMeta, error) {
+func (*client) WatchKeyspaces(context.Context) (chan []*keyspacepb.KeyspaceMeta, error) {
 	return nil, errors.Errorf("WatchKeyspaces unimplemented")
 }