Skip to content

Commit

Permalink
This is an automated cherry-pick of pingcap#52179
Browse files Browse the repository at this point in the history
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
  • Loading branch information
guo-shaoge authored and ti-chi-bot committed Mar 28, 2024
1 parent 7a5f72e commit 6360e24
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 0 deletions.
63 changes: 63 additions & 0 deletions pkg/util/topsql/reporter/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "reporter",
srcs = [
"datamodel.go",
"datasink.go",
"pubsub.go",
"reporter.go",
"single_target.go",
],
importpath = "github.com/pingcap/tidb/pkg/util/topsql/reporter",
visibility = ["//visibility:public"],
deps = [
"//pkg/config",
"//pkg/util",
"//pkg/util/hack",
"//pkg/util/logutil",
"//pkg/util/topsql/collector",
"//pkg/util/topsql/reporter/metrics",
"//pkg/util/topsql/state",
"//pkg/util/topsql/stmtstats",
"@com_github_pingcap_errors//:errors",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_pingcap_tipb//go-tipb",
"@com_github_wangjohn_quickselect//:quickselect",
"@org_golang_google_grpc//:grpc",
"@org_golang_google_grpc//backoff",
"@org_golang_google_grpc//credentials/insecure",
"@org_uber_go_atomic//:atomic",
"@org_uber_go_zap//:zap",
],
)

go_test(
name = "reporter_test",
timeout = "short",
srcs = [
"datamodel_test.go",
"datasink_test.go",
"main_test.go",
"pubsub_test.go",
"reporter_test.go",
"single_target_test.go",
],
embed = [":reporter"],
flaky = True,
shard_count = 36,
deps = [
"//pkg/config",
"//pkg/testkit/testsetup",
"//pkg/util/topsql/collector",
"//pkg/util/topsql/reporter/mock",
"//pkg/util/topsql/state",
"//pkg/util/topsql/stmtstats",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_pingcap_tipb//go-tipb",
"@com_github_stretchr_testify//assert",
"@com_github_stretchr_testify//require",
"@org_golang_google_grpc//metadata",
"@org_uber_go_goleak//:goleak",
],
)
12 changes: 12 additions & 0 deletions util/topsql/reporter/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,15 @@ import (
"errors"
"time"

<<<<<<< HEAD:util/topsql/reporter/pubsub.go
"github.com/pingcap/tidb/util"
"github.com/pingcap/tidb/util/logutil"
=======
"github.com/pingcap/failpoint"
"github.com/pingcap/tidb/pkg/util"
"github.com/pingcap/tidb/pkg/util/logutil"
reporter_metrics "github.com/pingcap/tidb/pkg/util/topsql/reporter/metrics"
>>>>>>> 089cba4853d (pkg/util: fix panic caused by logging grpc err (#52179)):pkg/util/topsql/reporter/pubsub.go
"github.com/pingcap/tipb/go-tipb"
"go.uber.org/zap"
)
Expand Down Expand Up @@ -96,6 +103,10 @@ func (ds *pubSubDataSink) OnReporterClosing() {

func (ds *pubSubDataSink) run() error {
defer func() {
if r := recover(); r != nil {
// To catch panic when log grpc error. https://github.com/pingcap/tidb/issues/51301.
logutil.BgLogger().Error("[top-sql] got panic in pub sub data sink, just ignore", zap.Error(util.GetRecoverError(r)))
}
ds.registerer.Deregister(ds)
ds.cancel()
}()
Expand Down Expand Up @@ -132,6 +143,7 @@ func (ds *pubSubDataSink) run() error {
return ctx.Err()
}

failpoint.Inject("mockGrpcLogPanic", nil)
if err != nil {
logutil.BgLogger().Warn(
"[top-sql] pubsub datasink failed to send data to subscriber",
Expand Down
5 changes: 5 additions & 0 deletions util/topsql/reporter/pubsub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import (
"testing"
"time"

"github.com/pingcap/failpoint"
"github.com/pingcap/tipb/go-tipb"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/metadata"
)

Expand Down Expand Up @@ -85,6 +87,8 @@ func TestPubSubDataSink(t *testing.T) {
_ = ds.run()
}()

panicPath := "github.com/pingcap/tidb/pkg/util/topsql/reporter/mockGrpcLogPanic"
require.NoError(t, failpoint.Enable(panicPath, "panic"))
err := ds.TrySend(&ReportData{
DataRecords: []tipb.TopSQLRecord{{
SqlDigest: []byte("S1"),
Expand Down Expand Up @@ -117,4 +121,5 @@ func TestPubSubDataSink(t *testing.T) {
mockStream.Unlock()

ds.OnReporterClosing()
require.NoError(t, failpoint.Disable(panicPath))
}

0 comments on commit 6360e24

Please sign in to comment.