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.

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

IsInternalOperationTableName: see if a table is used internally by vitess #7104

Merged

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented Dec 3, 2020

The new function IsInternalOperationTableName(tableName string) receives some table name and heuristically answers whether this table stands for an internal Vitess operation table. Such tables may be:

  • artifacts by gh-ost (during/after migration)
  • artifacts by pt-online-schema-change (during/after migration)
  • tables in table GC flow

The immediate use case is for VStreamer to be able to ignore such tables.

Usage:

import "vitess.io/vitess/go/vt/schema"

if schema.IsInternalOperationTableName("mytable") {
 // ...
}

cc @rohit-nayak-ps

…tess

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

lgtm (flagged a couple of typos)

// - 1876a01a-354d-11eb-9a79-f8e4e33000bb (delimeter = "-")
// - 7cee19dd_354b_11eb_82cd_f875a4d24e90 (delimeter = "_")
// - 55d00cdce6ab11eabfe60242ac1c000d (delimeter = "")
func createUUID(delimeter string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: delimeter => delimiter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -0,0 +1,60 @@
/*
Copyright 2019 The Vitess Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a new file, right? Copyright should say 2020

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

onlineDdlUUIDRegexp = regexp.MustCompile(`^[0-f]{8}_[0-f]{4}_[0-f]{4}_[0-f]{4}_[0-f]{12}$`)
strategyParserRegexp = regexp.MustCompile(`^([\S]+)\s+(.*)$`)
onlineDDLGeneratedTableNameRegexp = regexp.MustCompile(`^_[0-f]{8}_[0-f]{4}_[0-f]{4}_[0-f]{4}_[0-f]{12}_([0-9]{14})_(gho|ghc|del|new)$`)
ptOSCGeneratedTableNameRegexp = regexp.MustCompile(`^_.*_old$`)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was a bit unsure at first about ignoring _.*_old tables since it looks like it has a higher probability of name clashes. But on the other hand it might be a cool VReplication feature: you can use this naming convention for deprecated tables which results in movetables/reshard automatically ignoring tables!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 and unfortunately pt-osc does not let you control this specific name; it always renames your_table to _your_table_old (with potential extra _ prefixes like ____your_table_old)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm interested in onlineDDLGeneratedTableNameRegexp and VStream API. If we can't upgrade Vitess to this version in production yet. Can I use onlineDDLGeneratedTableNameRegexp to explicitly filter out "gh-ost" internal tables from the VStream API, in order to avoid this type of schema reload issues? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that regexp will work correctly. I assume this is a temporary solution for you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Yes, it'll be a temporary solution until we upgrade our vitess to v9.0.0-rc1, which includes the feature this PR provides.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach merged commit ec0f891 into vitessio:master Dec 7, 2020
@shlomi-noach shlomi-noach deleted the internal-operation-table-name-check branch December 7, 2020 05:58
@askdba askdba added this to the v9.0 milestone Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants