simulator(dm): implement some core components #4838

dsdashun · 2022-03-10T06:54:35Z

What problem does this PR solve?

Issue Number: ref #4835

What is changed and how it works?

Implemented some core components of the simulator:

modification candidate pool (MCP)
random SQL generator based on unique keys

Check List

Tests

Unit test

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change

Side effects

Increased code complexity

Related changes

N/A

Release note

None

ti-chi-bot · 2022-03-10T06:54:36Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Ehco1996
lance6716

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.

dsdashun · 2022-03-10T07:32:18Z

/run-verify

codecov-commenter · 2022-03-10T07:50:51Z

Codecov Report

Merging #4838 (699c0db) into master (687e248) will decrease coverage by 0.0336%.
The diff coverage is 51.3988%.

Flag	Coverage Δ
cdc	`60.8452% <51.3988%> (+0.2466%)`	⬆️
dm	`52.3731% <ø> (-0.0965%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master      #4838        +/-   ##
================================================
- Coverage   56.1240%   56.0903%   -0.0337%     
================================================
  Files           522        523         +1     
  Lines         65325      68436      +3111     
================================================
+ Hits          36663      38386      +1723     
- Misses        25094      26287      +1193     
- Partials       3568       3763       +195

lance6716

will review later

dm/simulator/internal/mcp/mcp.go

dm/simulator/internal/mcp/mcp_test.go

dm/simulator/internal/mcp/mcp.go

dm/simulator/internal/mcp/uk.go

dm/simulator/internal/mcp/mcp.go

dm/simulator/internal/sqlgen/impl.go

dm/simulator/internal/sqlgen/impl_test.go

dm/simulator/internal/mcp/mcp_test.go

D3Hunter · 2022-03-14T07:22:28Z

dm/simulator/internal/mcp/mcp_test.go

+	}
+}
+
+func (s *testMCPSuite) TestMCPAddDeleteInParallel() {


since operations on MCP is serialized(always locked), i think this test is not needed, but it's ok to keep it.

dm/simulator/internal/mcp/mcp.go

GMHDBJD · 2022-03-14T07:27:45Z

dm/simulator/internal/mcp/mcp.go

+}
+
+// NextUK randomly picks a unique key in the MCP.
+func (mcp *ModificationCandidatePool) NextUK() *UniqueKey {


we may need to allocate some uks in batch if the performance is not satisfied

Considering that we have introduced tikv/pd dep, maybe we can reuse pd.GenerateTSO

GMHDBJD

How about add a sqlgen demo?

* Refined MCP and SQLGen implementations according to the PR comments * Also refined tests

* Added an extra UK

dsdashun · 2022-03-15T12:36:00Z

/run-verify

lance6716

will review sqlgen soon

dm/simulator/internal/mcp/mcp_test.go

dm/simulator/internal/mcp/uk.go

dm/simulator/internal/mcp/uk_test.go

dm/simulator/internal/mcp/errors.go

Ehco1996 · 2022-03-17T01:47:47Z

dm/simulator/internal/mcp/mcp.go

+}
+
+// NextUK randomly picks a unique key in the MCP.
+func (mcp *ModificationCandidatePool) NextUK() *UniqueKey {


Considering that we have introduced tikv/pd dep, maybe we can reuse pd.GenerateTSO

dm/simulator/internal/mcp/mcp.go

dm/simulator/internal/mcp/uk.go

dm/simulator/internal/sqlgen/sqlgen.go

lance6716

rest lgtm

dm/simulator/internal/sqlgen/impl.go

lance6716 · 2022-03-29T04:03:33Z

ping @dsdashun

lance6716 · 2022-04-11T08:41:19Z

What should we do about this PR @dsdashun

Ehco1996

LGTM

Ehco1996 · 2022-04-22T05:52:00Z

dm/simulator/internal/sqlgen/sqlgen.go

+)
+
+// SQLGenerator contains all the operations for generating SQLs.
+type SQLGenerator interface {


need a GenCreateTableSQL func to make sqlgen more friendly

maybe do this later

dsdashun · 2022-04-22T07:11:52Z

/merge

ti-chi-bot · 2022-04-22T07:11:54Z

@dsdashun: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

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 · 2022-04-22T07:14:08Z

/hold

I think we can merge it after the v6.1 LTS release

D3Hunter · 2022-04-24T12:38:26Z

/hold

I think we can merge it after the v6.1 LTS release

seems doesn't matter, this module is quite standalone

lance6716 · 2022-04-24T12:56:26Z

/unhold
just learned that code freeze for v6.1 is 5/19

lance6716 · 2022-04-24T12:56:33Z

/merge

ti-chi-bot · 2022-04-24T12:56:36Z

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

Commit hash: 08b991d

ti-chi-bot · 2022-04-25T07:39:00Z

@dsdashun: 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

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.

dsdashun added 2 commits March 10, 2022 14:38

simulator(dm): modification candidate pool implementation

93e859e

simulator(dm): sql generator implementation

e7e2386

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 10, 2022

dsdashun added the area/dm Issues or PRs related to DM. label Mar 10, 2022

dsdashun requested review from lance6716, D3Hunter and GMHDBJD March 10, 2022 06:56

lance6716 reviewed Mar 11, 2022

View reviewed changes

GMHDBJD reviewed Mar 14, 2022

View reviewed changes

dm/simulator/internal/mcp/mcp.go Outdated Show resolved Hide resolved

D3Hunter reviewed Mar 14, 2022

View reviewed changes

GMHDBJD reviewed Mar 14, 2022

View reviewed changes

dsdashun added 2 commits March 15, 2022 18:06

simulator(dm): refine MCP and sqlgen implementations

d16e994

* Refined MCP and SQLGen implementations according to the PR comments * Also refined tests

simulator(dm): mcp supports parallel calling of NextUK()

98ac4a1

* Added an extra UK

lance6716 reviewed Mar 16, 2022

View reviewed changes

dm/simulator/internal/mcp/mcp_test.go Show resolved Hide resolved

dm/simulator/internal/mcp/uk.go Outdated Show resolved Hide resolved

dm/simulator/internal/mcp/uk.go Outdated Show resolved Hide resolved

dm/simulator/internal/mcp/uk_test.go Outdated Show resolved Hide resolved

Ehco1996 reviewed Mar 17, 2022

View reviewed changes

simulator: some changes according to the comments

ed851b0

lance6716 reviewed Mar 21, 2022

View reviewed changes

dm/simulator/internal/sqlgen/impl.go Outdated Show resolved Hide resolved

dm/simulator/internal/sqlgen/impl.go Show resolved Hide resolved

dsdashun added 2 commits April 16, 2022 10:25

Merge remote-tracking branch 'origin/master' into simulator-pr

0d7e861

simulator: sqlgen improvements on NULL UK value & empty filters

08b991d

lance6716 approved these changes Apr 21, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 21, 2022

Ehco1996 approved these changes Apr 22, 2022

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 Apr 22, 2022

dsdashun assigned lance6716 Apr 22, 2022

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 22, 2022

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 24, 2022

ti-chi-bot added 2 commits April 24, 2022 20:56

Merge branch 'master' into simulator-pr

699c0db

Merge branch 'master' into simulator-pr

1eabcf6

ti-chi-bot merged commit 06f54d0 into pingcap:master Apr 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simulator(dm): implement some core components #4838

simulator(dm): implement some core components #4838

dsdashun commented Mar 10, 2022

ti-chi-bot commented Mar 10, 2022 •

edited

Loading

dsdashun commented Mar 10, 2022

codecov-commenter commented Mar 10, 2022 •

edited

Loading

lance6716 left a comment

D3Hunter Mar 14, 2022

GMHDBJD Mar 14, 2022

Ehco1996 Mar 17, 2022

GMHDBJD left a comment

dsdashun commented Mar 15, 2022

lance6716 left a comment

Ehco1996 Mar 17, 2022

lance6716 left a comment

lance6716 commented Mar 29, 2022

lance6716 commented Apr 11, 2022

Ehco1996 left a comment

Ehco1996 Apr 22, 2022

dsdashun commented Apr 22, 2022

ti-chi-bot commented Apr 22, 2022

lance6716 commented Apr 22, 2022

D3Hunter commented Apr 24, 2022

lance6716 commented Apr 24, 2022

lance6716 commented Apr 24, 2022

ti-chi-bot commented Apr 24, 2022

ti-chi-bot commented Apr 25, 2022

simulator(dm): implement some core components #4838

simulator(dm): implement some core components #4838

Conversation

dsdashun commented Mar 10, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Mar 10, 2022 • edited Loading

dsdashun commented Mar 10, 2022

codecov-commenter commented Mar 10, 2022 • edited Loading

Codecov Report

lance6716 left a comment

Choose a reason for hiding this comment

D3Hunter Mar 14, 2022

Choose a reason for hiding this comment

GMHDBJD Mar 14, 2022

Choose a reason for hiding this comment

Ehco1996 Mar 17, 2022

Choose a reason for hiding this comment

GMHDBJD left a comment

Choose a reason for hiding this comment

dsdashun commented Mar 15, 2022

lance6716 left a comment

Choose a reason for hiding this comment

Ehco1996 Mar 17, 2022

Choose a reason for hiding this comment

lance6716 left a comment

Choose a reason for hiding this comment

lance6716 commented Mar 29, 2022

lance6716 commented Apr 11, 2022

Ehco1996 left a comment

Choose a reason for hiding this comment

Ehco1996 Apr 22, 2022

Choose a reason for hiding this comment

dsdashun commented Apr 22, 2022

ti-chi-bot commented Apr 22, 2022

lance6716 commented Apr 22, 2022

D3Hunter commented Apr 24, 2022

lance6716 commented Apr 24, 2022

lance6716 commented Apr 24, 2022

ti-chi-bot commented Apr 24, 2022

ti-chi-bot commented Apr 25, 2022

ti-chi-bot commented Mar 10, 2022 •

edited

Loading

codecov-commenter commented Mar 10, 2022 •

edited

Loading