*: limit checker error at a finer granularity and auto increase max size #1624

lance6716 · 2021-04-22T08:21:29Z

What problem does this PR solve?

close #1478

What is changed and how it works?

when RPC returned grpc: received message larger than max (5242885 vs. 4194304), increase max message size and retry
update dependency to limit the warn/error count at a finer granularity, and pretty print precheck errors

Check List

Tests

Unit test

Related changes

Need to cherry-pick to the release branch
Need to be included in the release note

checker/checker.go

lichunzhu · 2021-04-25T09:15:14Z

dm/ctl/common/util.go

+	"github.com/pingcap/dm/pkg/log"

 	"github.com/pingcap/dm/dm/config"
 	"github.com/pingcap/dm/dm/pb"


I think this is caused by our make fmt @Ehco1996 PTAL

our fmt tools just can't group this imports correctly... but you can formart manually - -.

Can you fix these imports manually?

lichunzhu · 2021-04-25T09:21:48Z

dm/ctl/common/util.go

+	case codes.ResourceExhausted:
+		matches := re.FindStringSubmatch(err.Error())
+		if len(matches) == 3 {
+			msgSize, err2 := strconv.Atoi(matches[1])


Should we increase msgSize a little more?

unit test shows this is enough

Will the result become a little bigger for the second request?

we can't decide a reasonable delta for "a little bigger" 🤔

OK, I'm just afraid it's too strict sometimes.

Is there a hard limit of gRPC message size? And how about use next_pow_of_2(msg_size) as the new limit

Is there a hard limit of gRPC message size?

MaxInt. This only has memory affects. next_pow_of_2 seems more like a strategy for container capacity growing, while here I think using exactly the size is OK, because we sent the RPC again in a short time so the response is more likely not changed.

lichunzhu

LGTM

glorv · 2021-04-27T12:31:56Z

dm/master/server_test.go

+	//nolint:errcheck
+	go server.Serve(lis)
+
+	conn, err := grpc.Dial(utils.UnwrapScheme(masterAddr),


should add defer conn.Close() here? 🤔

lance6716 · 2021-04-28T02:21:15Z

/hold

lance6716 · 2021-04-28T02:24:05Z

/unhold

Ehco1996 · 2021-04-30T02:45:30Z

/lgtm

ti-chi-bot · 2021-04-30T02:45:32Z

@Ehco1996: /lgtm is only allowed for the reviewers in list.

In response to this:

/lgtm

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.

lance6716 · 2021-05-12T06:54:29Z

/cc @glorv

glorv · 2021-05-13T01:58:22Z

/lgtm

ti-chi-bot · 2021-05-13T01:58:26Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

glorv
lichunzhu

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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

lance6716 · 2021-05-13T02:11:35Z

/merge

ti-chi-bot · 2021-05-13T02:11:39Z

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

Commit hash: 62ce2a2

lance6716 · 2021-05-13T05:05:16Z

/merge

ti-chi-bot · 2021-05-13T05:05:19Z

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

Commit hash: bbc0ca1

Ehco1996 · 2021-05-13T05:51:28Z

/run-compatibility-test

ti-chi-bot · 2021-05-13T08:24:17Z

In response to a cherrypick label: new pull request created: #1688.

…ize (#1624) (#1688)

checker: update returning structure of checker

71a5959

ti-chi-bot added the do-not-merge/work-in-progress label Apr 22, 2021

ti-chi-bot requested review from GMHDBJD and lichunzhu April 22, 2021 08:21

ti-chi-bot added the size/M label Apr 22, 2021

fix CI and add TODO

06d151e

ti-chi-bot added size/L and removed size/M labels Apr 24, 2021

lance6716 force-pushed the update-checker branch 2 times, most recently from e8fd5f0 to 685baf2 Compare April 24, 2021 13:56

lance6716 changed the title ~~[WIP] checker: update returning structure of checker~~ *: limit checker error at a finer granularity and auto increase max size Apr 24, 2021

ti-chi-bot removed the do-not-merge/work-in-progress label Apr 24, 2021

lance6716 added needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Apr 24, 2021

lance6716 added this to the v2.0.3 milestone Apr 24, 2021

adjust grpc max message size on error

b7ac2ac

lance6716 force-pushed the update-checker branch from 685baf2 to b7ac2ac Compare April 24, 2021 14:01

lichunzhu reviewed Apr 25, 2021

View reviewed changes

lance6716 added 2 commits April 26, 2021 12:07

Merge branch 'master' of github.com:pingcap/dm into update-checker

6a40c2a

reorder imports

008ef69

lichunzhu approved these changes Apr 26, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 One reviewer already commented LGTM label Apr 26, 2021

glorv reviewed Apr 27, 2021

View reviewed changes

lance6716 added 2 commits April 28, 2021 10:19

Merge branch 'master' of github.com:pingcap/dm into update-checker

49de347

address comment

0df21ec

ti-chi-bot added the do-not-merge/hold label Apr 28, 2021

ti-chi-bot removed the do-not-merge/hold label Apr 28, 2021

Merge branch 'master' into update-checker

62ce2a2

lance6716 modified the milestones: v2.0.3, v2.0.4 May 8, 2021

ti-chi-bot requested a review from glorv May 12, 2021 06:54

ti-chi-bot added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels May 13, 2021

ti-chi-bot added the status/can-merge label May 13, 2021

ti-chi-bot and others added 2 commits May 13, 2021 10:11

Merge branch 'master' into update-checker

d9b4e53

fix lint

bbc0ca1

ti-chi-bot removed the status/can-merge label May 13, 2021

ti-chi-bot added the status/can-merge label May 13, 2021

Merge branch 'master' into update-checker

bc61377

Merge branch 'master' into update-checker

c2e370d

ti-chi-bot merged commit cd7b5d0 into pingcap:master May 13, 2021

ti-chi-bot mentioned this pull request May 13, 2021

*: limit checker error at a finer granularity and auto increase max size (#1624) #1688

Merged

ti-chi-bot added a commit that referenced this pull request May 14, 2021

*: limit checker error at a finer granularity and auto increase max s…

3fbe33b

…ize (#1624) (#1688)

lance6716 mentioned this pull request Jul 1, 2021

CDC-Cloud: owner exited with error "code = ResourceExhausted desc = trying to send message larger than max (7749468 vs. 2097152)" pingcap/tiflow#2190

Closed

lance6716 mentioned this pull request Sep 10, 2024

warn/error count limit for check-task/start-task has wrong granularity pingcap/tiflow#11587

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: limit checker error at a finer granularity and auto increase max size #1624

*: limit checker error at a finer granularity and auto increase max size #1624

lance6716 commented Apr 22, 2021 •

edited

Loading

lichunzhu Apr 25, 2021

lance6716 Apr 25, 2021 •

edited

Loading

Ehco1996 Apr 25, 2021

lichunzhu Apr 26, 2021

lichunzhu Apr 25, 2021

lance6716 Apr 25, 2021

lichunzhu Apr 25, 2021

lance6716 Apr 25, 2021 •

edited

Loading

lichunzhu Apr 26, 2021

glorv Apr 27, 2021

lance6716 Apr 27, 2021

lichunzhu left a comment

glorv Apr 27, 2021

lance6716 commented Apr 28, 2021

lance6716 commented Apr 28, 2021

Ehco1996 commented Apr 30, 2021

ti-chi-bot commented Apr 30, 2021

lance6716 commented May 12, 2021

glorv commented May 13, 2021

ti-chi-bot commented May 13, 2021

lance6716 commented May 13, 2021

ti-chi-bot commented May 13, 2021

lance6716 commented May 13, 2021

ti-chi-bot commented May 13, 2021

Ehco1996 commented May 13, 2021

ti-chi-bot commented May 13, 2021

*: limit checker error at a finer granularity and auto increase max size #1624

*: limit checker error at a finer granularity and auto increase max size #1624

Conversation

lance6716 commented Apr 22, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Choose a reason for hiding this comment

lance6716 Apr 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lance6716 Apr 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lichunzhu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lance6716 commented Apr 28, 2021

lance6716 commented Apr 28, 2021

Ehco1996 commented Apr 30, 2021

ti-chi-bot commented Apr 30, 2021

lance6716 commented May 12, 2021

glorv commented May 13, 2021

ti-chi-bot commented May 13, 2021

lance6716 commented May 13, 2021

ti-chi-bot commented May 13, 2021

lance6716 commented May 13, 2021

ti-chi-bot commented May 13, 2021

Ehco1996 commented May 13, 2021

ti-chi-bot commented May 13, 2021

lance6716 commented Apr 22, 2021 •

edited

Loading

lance6716 Apr 25, 2021 •

edited

Loading

lance6716 Apr 25, 2021 •

edited

Loading