Online DDL: DROP TABLE translated to RENAME TABLE statement

[shlomi-noach]

Description

Starting #7083, CREATE and DROP tables can execute as Online-DDL, meaning they're scheduled, audited and executed by tablet servers.

With this PR, a DROP TABLE statement is not only treated as online DDL, it is actually converted to a RENAME TABLE statement, to work with the table lifecycle mechanism.

The table is renamed into something like _vt_HOLD_b0d1fb34450a11ebb980f875a4d24e90_20201226103654. By just having that kind of name, the table is later garbage-collected by the lifecycle mechanism, to be purged->evacuated->dropped.

We take care of DROP TABLE IF EXISTS by checking the return code from the RENAME TABLE statement, and ignoring the specific error that indicates the table does not exist.

Also in this PR, CREATE and DROP are supported through vtgate. Previously they were only supported by vtctl.

Last, also in this PR, is retaining the GC UUID (b0d1fb34450a11ebb980f875a4d24e90 in the above _vt_HOLD_b0d1fb34450a11ebb980f875a4d24e90_20201226103654 example) such that:

• The initial UUID is based on the Online DDL UUID. e.g.:

mysql> drop table tt;
+--------------------------------------+
| uuid                                 |
+--------------------------------------+
| b0d1fb34_450a_11eb_b980_f875a4d24e90 |
+--------------------------------------+

... time passes ...

mysql> show tables;
+----------------------------------------------------------+
| Tables_in_vt_commerce                                    |
+----------------------------------------------------------+
| _vt_HOLD_b0d1fb34450a11ebb980f875a4d24e90_20201226103654 |
| corder                                                   |
| customer                                                 |
| product                                                  |
+----------------------------------------------------------+

• transitioning into next state, same UUID is reused, e.g. that table will transform into _vt_PURGE_b0d1fb34450a11ebb980f875a4d24e90_20201230103654
• online ALTER TABLE artifacts are similarly named after the Online DDL UUID

There can be no conflict of naming because both state (HOLD, PURGE, etc.) as well as timestamps differ, even when using same UUID.

Related Issue(s)

tracking: #6689, #6926
Related: #7083
Depends on: #7151

Checklist

• Should this PR be backported?
• Tests were added or are not required
• Documentation was added or is not required

Impacted Areas in Vitess

Components that this PR will affect:

• Query Serving
• VReplication
• Cluster Management
• Build

[shlomi-noach]

Depends on: #7151

[shlomi-noach]

Documentation PR to follow.

[shlomi-noach]

Request for review

[rohit-nayak-ps]

lgtm

[shlomi-noach]

Merge conflicts are due to #7235
See proposed solution in #7247

[systay]

:nit: :rant: This is confusing code. I know the old code did this, but we should fix it while we are in here. ddlStmt and action have not been assigned to here, so this is equivalent to

return nil, nil, fmt.Errorf("Error parsing statement: SQL=%s, error=%+v", sql, err)

I would suggest to not use named return arguments unless they help readability or need to be changed by a defer.

[shlomi-noach]

I personally find this to be more idiomatic, because I don't need to think about the actual datatype for those return variables and I don't need to think about what's the default return value -- it's implicitly done on my behalf. Also, to me, it more readable to see what's being returned semantically.

Having said that I don't feel strongly and I'll adapt per your review.

[shlomi-noach]

resolved

[systay]

You are right to point out that confusing is highly subjective.

Let me explain where I'm coming from. I've always felt that it's important to return empty or nil values from a function when an error occurs. Some functions allow you to return both a valid value and an error (example: https://golang.org/src/io/io.go?s=11741:11795#L322), so I always make sure to explicitly return nil or empty values when the function doesn't have a valid value to return.

The named return argument pattern makes it a little more difficult for a casual reader to see if we are returning the result as far as we got when the error occurred, or an empty value to signal that nothing was actually done.

[shlomi-noach]

The way I see it, when an error is returned, in vast majority of cases the other values do not matter (are expected to be ignored). Therefore, it is only a rare occasion where those values will be checked. To me the code is more readable where I understand what it is that I'm returning semantically (e.g. this is the uuid rather than ""). And it's only the rare occasions where I need to worry about the actual values returned.

So I guess my confusion is the inversion of yours...

[shlomi-noach]

fixing failed tests after merging conflicts

[shlomi-noach]

endtoend tests fixed

[shlomi-noach]

to be followed by a documentation PR

[shlomi-noach]

documentation added and merged in vitessio/website#663