pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL #8069

amyangfei · 2023-01-12T09:43:12Z

What problem does this PR solve?

Issue Number: ref #8057
ref #8084

Task-2

What is changed and how it works?

Use raw string format to generate update sql.
Note this PR doesn't replace GenUpdate with GenUpdateFast, we will use GenUpdateFast after more correctness check and test.
Benchmark result is as following, the CPU costs decreases to 10%-16%, memory allocation decreases to 5%-11%. (Horizontal axis denotes the batch size of RowChange)

CPU time	memory allocation

➜  go test -run='^$' -benchmem -bench '^(BenchmarkGenUpdate)$' github.com/pingcap/tiflow/pkg/sqlmodel
goos: darwin
goarch: arm64
pkg: github.com/pingcap/tiflow/pkg/sqlmodel
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch1-8              162788              7273 ns/op            8145 B/op        167 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch2-8              114633             10409 ns/op           10034 B/op        227 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch4-8               76413             15544 ns/op           13747 B/op        346 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch8-8               46393             25777 ns/op           20724 B/op        583 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch16-8              25616             46324 ns/op           33784 B/op       1056 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch32-8              13632             87381 ns/op           62977 B/op       2002 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch64-8               6699            169962 ns/op          128919 B/op       3893 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQL-Batch128-8              3602            331583 ns/op          246086 B/op       7671 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch1-8          914386              1224 ns/op            1712 B/op         20 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch2-8          789146              1451 ns/op            2168 B/op         23 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch4-8          615506              1810 ns/op            3160 B/op         29 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch8-8          439963              2655 ns/op            5056 B/op         41 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch16-8         229112              5016 ns/op           12570 B/op         67 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch32-8         123585              9502 ns/op           27069 B/op        117 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch64-8          61982             19472 ns/op           63620 B/op        216 allocs/op
BenchmarkGenUpdate/OneColumnPK-GenUpdateSQLFast-Batch128-8         32689             36418 ns/op          122018 B/op        410 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch1-8           159885              7308 ns/op            8145 B/op        167 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch2-8           114904             10370 ns/op           10034 B/op        227 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch4-8            77086             15542 ns/op           13747 B/op        346 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch8-8            46633             25813 ns/op           20724 B/op        583 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch16-8           25890             46306 ns/op           33784 B/op       1056 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch32-8           13792             87409 ns/op           62976 B/op       2002 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch64-8            6789            169864 ns/op          128918 B/op       3893 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQL-Batch128-8           3518            340506 ns/op          246086 B/op       7671 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch1-8       958459              1223 ns/op            1712 B/op         20 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch2-8       781336              1457 ns/op            2168 B/op         23 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch4-8       653800              1812 ns/op            3160 B/op         29 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch8-8       447306              2660 ns/op            5056 B/op         41 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch16-8      236116              5015 ns/op           12570 B/op         67 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch32-8      125460              9503 ns/op           27069 B/op        117 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch64-8       62330             19395 ns/op           63620 B/op        216 allocs/op
BenchmarkGenUpdate/MultiColumnsPK-GenUpdateSQLFast-Batch128-8              32689             36641 ns/op          122018 B/op        410 allocs/op
PASS
ok      github.com/pingcap/tiflow/pkg/sqlmodel  44.062s

Check List

Tests

Unit test
Integration test

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

None

ti-chi-bot · 2023-01-12T09:43:14Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

asddongmen
hi-rustin

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Rustin170506 · 2023-01-13T03:16:04Z

/cc

nongfushanquan · 2023-01-16T07:00:51Z

pkg/sqlmodel/multirow.go

+// Input `changes` should have same target table and same columns for WHERE
+// (typically same PK/NOT NULL UK), otherwise the behaviour is undefined.
+// It is a faster version compared with GenUpdateSQL.
+func GenUpdateSQLFast(changes ...*RowChange) (string, []interface{}) {


Should the batch size be limited ?

I have summited a related issue #8084, and this limit will be added in another PR.

Rustin170506

rest LGTM. Thanks!

pkg/sqlmodel/multirow.go

Rustin170506 · 2023-01-18T06:31:30Z

pkg/sqlmodel/multirow.go

+// It is a faster version compared with GenUpdateSQL.
+func GenUpdateSQLFast(changes ...*RowChange) (string, []interface{}) {
+	if len(changes) == 0 {
+		log.L().DPanic("row changes is empty")


It seems we always use log.Panic() in TiCDC. Why do we need to get the current logger here? I'm not quite familiar with this API.

It keeps the same DPanic usage as GenUpdateSQL, since this function is also used in DM. If we don't panic here(not in development mode), row changes will be lost.
@lance6716 Is it ok to change to log.Panic() here for DM?

I think it's OK to use log.Panic.

@hi-rustin in DM when the log level is debug, we turn on Development flag.

tiflow/dm/pkg/log/log.go

Lines 107 to 118 in 171e21b

inDev := strings.ToLower(cfg.Level) == "debug"

// init DM logger

logger, props, err := pclog.InitLogger(&pclog.Config{

Level: cfg.Level,

Format: cfg.Format,

File: pclog.FileLogConfig{

Filename: cfg.File,

MaxSize: cfg.FileMaxSize,

MaxDays: cfg.FileMaxDays,

MaxBackups: cfg.FileMaxBackups,

},

Development: inDev,

and DM integration tests always use debug level log, so we can detect the misuse in developement and control the blast radius when the bug is really happened at user's side. But it's not important.

Changed to log.Panic()

amyangfei · 2023-01-18T09:35:32Z

/merge

ti-chi-bot · 2023-01-18T09:35:37Z

This pull request has been accepted and is ready to merge.

Commit hash: 89b7b23

ti-chi-bot · 2023-01-18T09:48:01Z

@amyangfei: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

trigger some heavy tests which will not run always when PR updated.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

amyangfei · 2023-01-18T10:13:06Z

/run-engine-integration-test

…pingcap#8069)

…pingcap#8069) ref pingcap#8057

ref #8084

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL

099b674

amyangfei added the area/ticdc Issues or PRs related to TiCDC. label Jan 12, 2023

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 12, 2023

fix make check

5168a5a

asddongmen self-requested a review January 13, 2023 02:28

ti-chi-bot requested a review from Rustin170506 January 13, 2023 03:16

nongfushanquan reviewed Jan 16, 2023

View reviewed changes

amyangfei added the status/ptal Could you please take a look? label Jan 17, 2023

asddongmen approved these changes Jan 18, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 18, 2023

Rustin170506 approved these changes Jan 18, 2023

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 18, 2023

amyangfei added 3 commits January 18, 2023 17:15

address comments

616a838

replace log.DPanic with log.Panic

985de55

Merge branch 'master' into optimize-gen-update-3

89b7b23

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 18, 2023

Merge branch 'master' into optimize-gen-update-3

7eee5e7

ti-chi-bot merged commit 7dddcd0 into pingcap:master Jan 18, 2023

amyangfei added a commit to ti-chi-bot/tiflow that referenced this pull request Feb 15, 2023

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL (…

756c7fe

…pingcap#8069)

qiancai mentioned this pull request Feb 16, 2023

releases: add 6.6.0 release notes pingcap/docs-cn#12652

Merged

14 tasks

amyangfei deleted the optimize-gen-update-3 branch April 3, 2023 07:51

amyangfei added a commit to amyangfei/tiflow that referenced this pull request Apr 3, 2023

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL (…

a22f4b7

…pingcap#8069) ref pingcap#8057

amyangfei mentioned this pull request Apr 3, 2023

cherry pick batch update optimization (#8069) (#8191) (#8452) #8727

Merged

ti-chi-bot pushed a commit that referenced this pull request Apr 13, 2023

cherry pick batch update optimization (#8069) (#8191) (#8452) (#8727)

95b5db3

ref #8084

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL #8069

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL #8069

amyangfei commented Jan 12, 2023 •

edited

Loading

ti-chi-bot commented Jan 12, 2023 •

edited

Loading

Rustin170506 commented Jan 13, 2023

nongfushanquan Jan 16, 2023

amyangfei Jan 16, 2023 •

edited

Loading

Rustin170506 left a comment

Rustin170506 Jan 18, 2023

amyangfei Jan 18, 2023

lance6716 Jan 18, 2023 •

edited

Loading

amyangfei Jan 18, 2023

amyangfei commented Jan 18, 2023

ti-chi-bot commented Jan 18, 2023

ti-chi-bot commented Jan 18, 2023

amyangfei commented Jan 18, 2023

	inDev := strings.ToLower(cfg.Level) == "debug"
	// init DM logger
	logger, props, err := pclog.InitLogger(&pclog.Config{
	Level: cfg.Level,
	Format: cfg.Format,
	File: pclog.FileLogConfig{
	Filename: cfg.File,
	MaxSize: cfg.FileMaxSize,
	MaxDays: cfg.FileMaxDays,
	MaxBackups: cfg.FileMaxBackups,
	},
	Development: inDev,

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL #8069

pkg(ticdc): add a faster raw string rendering version of GenUpdateSQL #8069

Conversation

amyangfei commented Jan 12, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

ti-chi-bot commented Jan 12, 2023 • edited Loading

Rustin170506 commented Jan 13, 2023

nongfushanquan Jan 16, 2023

Choose a reason for hiding this comment

amyangfei Jan 16, 2023 • edited Loading

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 Jan 18, 2023

Choose a reason for hiding this comment

amyangfei Jan 18, 2023

Choose a reason for hiding this comment

lance6716 Jan 18, 2023 • edited Loading

Choose a reason for hiding this comment

amyangfei Jan 18, 2023

Choose a reason for hiding this comment

amyangfei commented Jan 18, 2023

ti-chi-bot commented Jan 18, 2023

ti-chi-bot commented Jan 18, 2023

amyangfei commented Jan 18, 2023

amyangfei commented Jan 12, 2023 •

edited

Loading

ti-chi-bot commented Jan 12, 2023 •

edited

Loading

amyangfei Jan 16, 2023 •

edited

Loading

lance6716 Jan 18, 2023 •

edited

Loading