ddl: add sync bundles logic for creating and droping #28037

AilinKid · 2021-09-14T09:09:23Z

Signed-off-by: ailinkid 314806019@qq.com

What problem does this PR solve?

Issue Number: close #26580, #28058

Create table with placement policy or direct placement options should sync PD about the rules.
Drop table with placement policy or direct placement options should clean PD about the rules.
Also add a cache map about the policy reference relationship

What is changed and how it works?

What's Changed:

Create table with placement policy or direct placement options should sync PD about the rules.
Drop table with placement policy or direct placement options should clean PD about the rules.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

Release note

None

ti-chi-bot · 2021-09-14T09:09:24Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lcwangchao
xhebox

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.

sre-bot · 2021-09-14T09:14:50Z

Please follow PR Title Format:

pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

*: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

infoschema/infoschema.go

ddl/placement_policy.go

domain/domain.go

ddl/placement_policy.go

ddl/table.go

ddl/placement_policy.go

infoschema/infoschema.go

xhebox

My biggest concern is about policyDependencies in infoschema. Infoschema is neither persistent, nor strong consistent. It is tricky to use it broadcast events. It becomes the new all-in-one cache while it is designed to sync our old table/pt/db meta changes.

In fact, I wanted to remove all tricky bundle sync in infoschema by persisting more things on TiKV. But it is gone due to cyclic import between parser and TiDB.

Maybe storing ids in the meta info of policy is good and fast enough. Something like push another DDL job that will add affected ids into policy meta when binding a policy.

ddl/placement_policy.go

ddl/table.go

ddl/placement_policy.go

ddl/table.go

ddl/placement_policy_test.go

infoschema/infoschema.go

ddl/placement_policy.go

Signed-off-by: ailinkid <314806019@qq.com>

ddl/placement_policy.go

lcwangchao · 2021-09-22T04:19:02Z

ddl/placement_policy.go

+		}
+		// Do the http request only when the rules is existed.
+		bundles := make([]*placement.Bundle, 0, len(ids))
+		for _, id := range ids {


The id list contains database ids. However db do not have records stored in tikv

Nice insight, the work is also a preparation for what is coming from @zhaoxugang / @mattias who are under the db policy work

lcwangchao · 2021-09-22T04:21:12Z

ddl/table.go

+	}
+	if bundle == nil {
+		// get the default bundle from DB or PD.
+		bundle = infoschema.GetBundle(d.infoCache.GetLatest(), []int64{schemaID})


Why not generating the default bundle using the schema's meta.

yes we can, copyed from wanghe's work for simplicity.
cases is that: once schema did not set a policy, here may need a default bundle with PDBundleID (also from cache or via http to pd).

cache problem may be your concern, I will change this when all to-do things are much more clear.

AilinKid · 2021-09-22T07:07:28Z

/run-all-tests

Signed-off-by: ailinkid <314806019@qq.com>

… table id Signed-off-by: ailinkid <314806019@qq.com>

Signed-off-by: ailinkid <314806019@qq.com>

lcwangchao · 2021-09-23T10:07:05Z

Some logics are still need to be refactor because this implement is based on the legacy code. The refactor can be done in later pull requests.

xhebox · 2021-09-23T16:13:35Z

/merge

ti-chi-bot · 2021-09-23T16:13:38Z

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

Commit hash: 078fe50

nolouch · 2021-09-26T06:50:34Z

Why need to add a rule to PD when creating a table?

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 14, 2021

AilinKid changed the title ~~add the policy cache and sync logic~~ ddl: add the policy cache and sync logic for create and drop policy Sep 14, 2021

AilinKid changed the title ~~ddl: add the policy cache and sync logic for create and drop policy~~ ddl: add the policy cache and sync bundles logic for create and drop policy Sep 15, 2021

AilinKid changed the title ~~ddl: add the policy cache and sync bundles logic for create and drop policy~~ ddl: add the policy dependency cache and sync bundles logic for create and drop policy Sep 15, 2021

AilinKid changed the title ~~ddl: add the policy dependency cache and sync bundles logic for create and drop policy~~ ddl: add the policy dependency cache and sync bundles logic for create and drop Sep 15, 2021

xhebox self-requested a review September 15, 2021 05:33

lcwangchao linked an issue Sep 15, 2021 that may be closed by this pull request

Placement policy will be lost after restarting the tidb server #28058

Closed

AilinKid mentioned this pull request Sep 15, 2021

ddl: forbid dropping placement policy which is in use #28057

Merged

1 task

lcwangchao reviewed Sep 15, 2021

View reviewed changes

xhebox reviewed Sep 15, 2021

View reviewed changes

ddl/placement_policy.go Outdated Show resolved Hide resolved

ddl/table.go Outdated Show resolved Hide resolved

ddl/placement_policy.go Outdated Show resolved Hide resolved

ddl/table.go Outdated Show resolved Hide resolved

ddl/table.go Outdated Show resolved Hide resolved

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 16, 2021

AilinKid force-pushed the drop-policy-apply-table branch from 6314266 to 1cace4a Compare September 17, 2021 08:48

AilinKid requested review from lcwangchao and xhebox September 17, 2021 09:11

lcwangchao reviewed Sep 17, 2021

View reviewed changes

ddl/placement_policy_test.go Show resolved Hide resolved

lcwangchao reviewed Sep 17, 2021

View reviewed changes

infoschema/infoschema.go Outdated Show resolved Hide resolved

lcwangchao reviewed Sep 17, 2021

View reviewed changes

ddl/placement_policy.go Show resolved Hide resolved

AilinKid added 9 commits September 18, 2021 16:48

add the policy cache and sync logic

9f5aff4

Signed-off-by: ailinkid <314806019@qq.com>

fix the detach policy dependency

6985eae

Signed-off-by: ailinkid <314806019@qq.com>

add alter policy should revise all the depended ids

96e57ec

Signed-off-by: ailinkid <314806019@qq.com>

remove redundant code

78f54fb

Signed-off-by: ailinkid <314806019@qq.com>

defer

1ec2752

Signed-off-by: ailinkid <314806019@qq.com>

add test

b8bbc22

Signed-off-by: ailinkid <314806019@qq.com>

rm policy dependency

ad73f7b

Signed-off-by: ailinkid <314806019@qq.com>

rebase

db88515

Signed-off-by: ailinkid <314806019@qq.com>

.

70b8588

Signed-off-by: ailinkid <314806019@qq.com>

AilinKid changed the title ~~ddl: add sync bundles logic for create and drop~~ ddl: add sync bundles logic for creating and droping Sep 22, 2021

AilinKid added 3 commits September 22, 2021 10:45

fix test for show create table with placement policy

c45998b

Signed-off-by: ailinkid <314806019@qq.com>

Merge branch 'master' into drop-policy-apply-table

032d06a

Merge branch 'master' into drop-policy-apply-table

4e0d7ad

AilinKid requested a review from lcwangchao September 22, 2021 03:17

xhebox approved these changes Sep 22, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 22, 2021

lcwangchao reviewed Sep 22, 2021

View reviewed changes

Merge branch 'master' into drop-policy-apply-table

34ad749

AilinKid added 3 commits September 22, 2021 16:09

Merge branch 'master' into drop-policy-apply-table

88b081b

remove the dbs when reset the bundle

de6fef3

Signed-off-by: ailinkid <314806019@qq.com>

change the table level rule to cover the all the range with specified…

21bb88a

… table id Signed-off-by: ailinkid <314806019@qq.com>

AilinKid force-pushed the drop-policy-apply-table branch from 656800f to 21bb88a Compare September 23, 2021 04:14

AilinKid and others added 5 commits September 23, 2021 12:14

Merge branch 'master' into drop-policy-apply-table

a7766a7

Merge branch 'master' into drop-policy-apply-table

fc17d51

fix the test

d08239e

Signed-off-by: ailinkid <314806019@qq.com>

Merge branch 'master' into drop-policy-apply-table

b6ae1cc

Merge branch 'master' into drop-policy-apply-table

078fe50

lcwangchao approved these changes Sep 23, 2021

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 Sep 23, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 23, 2021

Merge branch 'master' into drop-policy-apply-table

44c2283

ti-chi-bot merged commit d14566c into pingcap:master Sep 23, 2021

nolouch mentioned this pull request Sep 26, 2021

Many useless rules and group-bundle is not override #28353

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: add sync bundles logic for creating and droping #28037

ddl: add sync bundles logic for creating and droping #28037

AilinKid commented Sep 14, 2021 •

edited

Loading

ti-chi-bot commented Sep 14, 2021 •

edited

Loading

sre-bot commented Sep 14, 2021

xhebox left a comment

lcwangchao Sep 22, 2021

AilinKid Sep 22, 2021 •

edited

Loading

lcwangchao Sep 22, 2021

AilinKid Sep 22, 2021 •

edited

Loading

AilinKid commented Sep 22, 2021

lcwangchao commented Sep 23, 2021

xhebox commented Sep 23, 2021

ti-chi-bot commented Sep 23, 2021

nolouch commented Sep 26, 2021

ddl: add sync bundles logic for creating and droping #28037

ddl: add sync bundles logic for creating and droping #28037

Conversation

AilinKid commented Sep 14, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Sep 14, 2021 • edited Loading

sre-bot commented Sep 14, 2021

xhebox left a comment

Choose a reason for hiding this comment

lcwangchao Sep 22, 2021

Choose a reason for hiding this comment

AilinKid Sep 22, 2021 • edited Loading

Choose a reason for hiding this comment

lcwangchao Sep 22, 2021

Choose a reason for hiding this comment

AilinKid Sep 22, 2021 • edited Loading

Choose a reason for hiding this comment

AilinKid commented Sep 22, 2021

lcwangchao commented Sep 23, 2021

xhebox commented Sep 23, 2021

ti-chi-bot commented Sep 23, 2021

nolouch commented Sep 26, 2021

AilinKid commented Sep 14, 2021 •

edited

Loading

ti-chi-bot commented Sep 14, 2021 •

edited

Loading

AilinKid Sep 22, 2021 •

edited

Loading

AilinKid Sep 22, 2021 •

edited

Loading