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

backupccl: crash on SHOW BACKUP for database with table referencing type in other database #55772

Closed
thoszhang opened this issue Oct 20, 2020 · 0 comments · Fixed by #55774
Closed
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.

Comments

@thoszhang
Copy link
Contributor

thoszhang commented Oct 20, 2020

We have a bug that makes cross-database type references possible (#55709 (comment)). This makes backups not work with RESTORE or SHOW BACKUP. SHOW BACKUP will crash the node:

root@:26257/defaultdb> create type t as enum('foo');
CREATE TYPE

Time: 93ms total (execution 93ms / network 0ms)

root@:26257/defaultdb> create table tbl (a t);
CREATE TABLE

Time: 198ms total (execution 78ms / network 120ms)

root@:26257/defaultdb> create database otherdb;
CREATE DATABASE

Time: 56ms total (execution 56ms / network 0ms)

root@:26257/defaultdb> alter table tbl rename to otherdb.tbl;
NOTICE: renaming tables with a qualification is deprecated
HINT: use ALTER TABLE tbl RENAME TO tbl instead
RENAME TABLE

Time: 185ms total (execution 57ms / network 128ms)

root@:26257/defaultdb> backup otherdb to 'nodelocal://backup';
ERROR: failed to resolve targets specified in the BACKUP stmt: table "otherdb" does not exist
root@:26257/defaultdb> backup database otherdb to 'nodelocal://backup';
ERROR: host component of nodelocal URI must be a node ID: nodelocal://backup
root@:26257/defaultdb> backup database otherdb to 'nodelocal://1/backup';
        job_id       |  status   | fraction_completed | rows | index_entries | bytes
---------------------+-----------+--------------------+------+---------------+--------
  600147520773095425 | succeeded |                  1 |    0 |             0 |     0
(1 row)

Time: 366ms total (execution 366ms / network 0ms)

root@:26257/defaultdb> restore otherdb from 'nodelocal://1/backup' with into_db=newdb;
ERROR: failed to resolve targets in the BACKUP location specified by the RESTORE stmt, use SHOW BACKUP to find correct targets: type "t" has unknown ParentID 50
root@:26257/defaultdb> show backup 'nodelocal://1/backup';
ERROR: driver: bad connection
warning: connection lost!
opening new connection: all session settings will be lost
warning: error retrieving the transaction status: dial tcp [::1]:26257: connect: connection refused
warning: connection lost!
opening new connection: all session settings will be lost
warning: error retrieving the database name: dial tcp [::1]:26257: connect: connection refused
@thoszhang thoszhang added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-disaster-recovery labels Oct 20, 2020
@thoszhang thoszhang added branch-release-20.2 release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Oct 20, 2020
craig bot pushed a commit that referenced this issue Oct 20, 2020
55459: kv: increase defaultRaftLogTruncationThreshold to 16MB r=nvanbenschoten a=nvanbenschoten

In v20.1, we increased the default max range size from 64MB to 512MB. However, we only doubled (258b965) the default raft log truncation threshold. This has the potential to exacerbate issues like #37906, because each range will now handle on average 8 times more write load, and will therefore be forced to truncate its log on average 4 times as frequently as it had previously.

This commit bumps the default raft log truncation to 16MB. It doesn't go as far as scaling the log truncation threshold by the max range size (either automatically or with a fixed 32MB default) because the cost of an individual log truncation is proportional to the log size and we don't want to make each log truncation significantly more disruptive. 16MB seems like a good middle ground and we know that we have customers already successfully running in production with this value.

55660: sql: add constraint name to constraint error r=yuzefovich a=alex-berger

Add constraint name to error for unique constraint, check constraintand foreign key constraint violations. Those constraint names are then propagated over the PostgreSQL wire protocol and will show up for example in JDBC exceptions (org.postgresql.util.PSQLException#getServerErrorMessage().getConstraint()). This commit improves PostgreSQL compatibility.

Release note: Improve PosgreSQL wire protocol compatibility by adding constraint name to sql errors.

55759: roachtest: don't expect node deaths in tpccbench r=nvanbenschoten a=nvanbenschoten

Closes #55542.

This was causing node deaths to be initially ignored in test
failures like #55542. The monitor was not watching when the
cluster was restarted, so there was no need to inform it of
the restart. Because of this, the monitor had an accumulated
"death quota" that allowed it to ignore the first few deaths
during the actual run of the test.

I tested this 5 times without issue.

55779: sql: disallow moving tables with user-defined types into a different DB r=lucy-zhang a=lucy-zhang

We disallow columns using user-defined types in a different database
from the table at creation time. However, we have a loophole where it's
possible to move the table to a different database using `RENAME` and
thus create a cross-database reference. This breaks backup/restore and
is generally unintended. This PR adds a check to disallow this.

Partially addresses #55709.
Related to #55772.

Release note (bug fix): Tables can no longer be moved to a different
database using `RENAME` if they have columns using user-defined types
(enums).

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Lucy Zhang <[email protected]>
@craig craig bot closed this as completed in 738848e Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant