Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl, stat: switch to new struct for remove partition, flash back and remove DDLEvent #55973

Merged
merged 19 commits into from
Sep 11, 2024
Merged

ddl, stat: switch to new struct for remove partition, flash back and remove DDLEvent #55973

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
89150c7
replace
fzzf678 Sep 10, 2024
de7ef46
rename field
fzzf678 Sep 10, 2024
e5f3826
remove useless func
fzzf678 Sep 10, 2024
22cfd23
replace DDLEvent
fzzf678 Sep 10, 2024
d7019e3
remove DDLEvent
fzzf678 Sep 10, 2024
5cf10ff
sort import
fzzf678 Sep 10, 2024
f4a2bba
update bazel
fzzf678 Sep 10, 2024
4813ed0
fix rename and line style
fzzf678 Sep 10, 2024
b117651
remove unused param
fzzf678 Sep 10, 2024
59a52a1
remove todo
fzzf678 Sep 10, 2024
f9175e4
print partition name when it's not empty
fzzf678 Sep 10, 2024
0f76ff3
supply comment
fzzf678 Sep 10, 2024
03a6621
add default branch
fzzf678 Sep 10, 2024
ece1cd2
||
fzzf678 Sep 10, 2024
67265d9
Update pkg/statistics/handle/ddl/ddl.go
fzzf678 Sep 10, 2024
7fd4d01
Update pkg/ddl/util/schema_change_notifier.go
fzzf678 Sep 10, 2024
627ff17
update comment
fzzf678 Sep 10, 2024
a04f0ca
log error in default branch
fzzf678 Sep 10, 2024
8b171a4
Update ddl.go
fzzf678 Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/statistics/handle/ddl/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ go_library(
"//pkg/statistics/handle/storage",
"//pkg/statistics/handle/types",
"//pkg/statistics/handle/util",
"//pkg/util/intest",
"@com_github_pingcap_errors//:errors",
"@org_uber_go_zap//:zap",
],
Expand Down
6 changes: 5 additions & 1 deletion pkg/statistics/handle/ddl/ddl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ import (
"github.com/pingcap/tidb/pkg/sessionctx"
"github.com/pingcap/tidb/pkg/sessionctx/variable"
"github.com/pingcap/tidb/pkg/statistics/handle/lockstats"
"github.com/pingcap/tidb/pkg/statistics/handle/logutil"
"github.com/pingcap/tidb/pkg/statistics/handle/storage"
"github.com/pingcap/tidb/pkg/statistics/handle/types"
"github.com/pingcap/tidb/pkg/statistics/handle/util"
"github.com/pingcap/tidb/pkg/util/intest"
"go.uber.org/zap"
)

type ddlHandlerImpl struct {
Expand Down Expand Up @@ -183,7 +186,8 @@ func (h *ddlHandlerImpl) HandleDDLEvent(s *ddlutil.SchemaChangeEvent) error {
case model.ActionFlashbackCluster:
return h.statsWriter.UpdateStatsVersion()
fzzf678 marked this conversation as resolved.
Show resolved Hide resolved
default:
return errors.Trace(errors.Errorf("unhandled DDL type %v", s.GetType()))
intest.Assert(false)
logutil.StatsLogger().Error("Unhandled schema change event", zap.Stringer("type", s.GetType()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to print the whole event here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What’s the purpose of printing the whole event in log?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought that the purpose of this default branch is to remind developer there is an unhandled event, any other use? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would be easier to figure out what information is in it. But I am okay with it if you only want to print the type here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL again 8b171a4

}
return nil
}
Expand Down