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 ca6a857 commit 0e62a2e
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 0 deletions.
10 changes: 10 additions & 0 deletions util/topsql/reporter/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,22 @@ go_test(
embed = [":reporter"],
flaky = True,
deps = [
<<<<<<< HEAD:util/topsql/reporter/BUILD.bazel
"//config",
"//testkit/testsetup",
"//util/topsql/collector",
"//util/topsql/reporter/mock",
"//util/topsql/state",
"//util/topsql/stmtstats",
=======
"//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",
>>>>>>> 089cba4853d (pkg/util: fix panic caused by logging grpc err (#52179)):pkg/util/topsql/reporter/BUILD.bazel
"@com_github_pingcap_tipb//go-tipb",
"@com_github_stretchr_testify//assert",
"@com_github_stretchr_testify//require",
Expand Down
12 changes: 12 additions & 0 deletions util/topsql/reporter/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,16 @@ import (
"errors"
"time"

<<<<<<< HEAD:util/topsql/reporter/pubsub.go
"github.com/pingcap/tidb/util"
"github.com/pingcap/tidb/util/logutil"
reporter_metrics "github.com/pingcap/tidb/util/topsql/reporter/metrics"
=======
"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 @@ -97,6 +104,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 @@ -133,6 +144,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 0e62a2e

Please sign in to comment.