Skip to content

Commit

Permalink
pkg/util: fix panic caused by logging grpc err (#52179) (#52187)
Browse files Browse the repository at this point in the history
close #51301
  • Loading branch information
ti-chi-bot authored Mar 28, 2024
1 parent d3e6a29 commit 69894ec
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 0 deletions.
1 change: 1 addition & 0 deletions util/topsql/reporter/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ go_test(
"//util/topsql/reporter/mock",
"//util/topsql/state",
"//util/topsql/stmtstats",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_pingcap_tipb//go-tipb",
"@com_github_stretchr_testify//assert",
"@com_github_stretchr_testify//require",
Expand Down
8 changes: 8 additions & 0 deletions util/topsql/reporter/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"errors"
"time"

tidberrors "github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/tidb/util"
"github.com/pingcap/tidb/util/logutil"
reporter_metrics "github.com/pingcap/tidb/util/topsql/reporter/metrics"
Expand Down Expand Up @@ -97,6 +99,11 @@ func (ds *pubSubDataSink) OnReporterClosing() {

func (ds *pubSubDataSink) run() error {
defer func() {
if r := recover(); r != nil {
err := tidberrors.Errorf("%v", r)
// 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(err))
}
ds.registerer.Deregister(ds)
ds.cancel()
}()
Expand Down Expand Up @@ -133,6 +140,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 69894ec

Please sign in to comment.