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

[YSQL] Tracking database dependencies on tablespaces #10487

Open
thepinetree opened this issue Nov 4, 2021 · 0 comments
Open

[YSQL] Tracking database dependencies on tablespaces #10487

thepinetree opened this issue Nov 4, 2021 · 0 comments
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue

Comments

@thepinetree
Copy link

thepinetree commented Nov 4, 2021

Jira Link: DB-1112

Description

Database dependencies on tablespaces are not being tracked, in which the following problematic scenario would succeed:

CREATE TABLESPACE foo;
CREATE DATABASE test_db WITH TABLESPACE foo;
DROP TABLESPACE foo;

while in Postgres, the scenario would not since the specified tablespace foo is not "empty" since it contains various system tables. Since in Yugabyte, we disassociate storage from tablespaces, we must still prevent tablespaces associated with databases from being dropped incorrectly. As such, adding a dependency of database on tablespace prevents this issue.

Status Feature
⬜️ Add default tablespace dependency tracking for databases created with CREATE DATABASE
⬜️ Add default tablespace dependency tracking for databases created on startup (postgres, template0, template1, yugabyte, system_platform)
⬜️ Add support for ALTER DATABASE SET TABLESPACE default tablespace dependency modification
@thepinetree thepinetree added the area/ysql Yugabyte SQL (YSQL) label Nov 4, 2021
@thepinetree thepinetree changed the title [YSQL] Add dependency tracking for databases spawned on startup [YSQL] Tracking database dependencies on tablespaces Nov 4, 2021
thepinetree pushed a commit that referenced this issue Nov 5, 2021
…titioned relations

Summary:
Adds additional support for tablespace dependencies based on [Postgres behavior](postgres/postgres@ebfe2db) by importing upstream Postgres commit with minor changes.

Major changes to the Postgres diff include:
* The replacement of the existing Yugabyte `recordDependencyOnTablespace` function to prefer a cleaner mode
* Skipping already applied changes in D10180
* The use of `heap_open` and `heap_close` instead of `table_open` and `table_close` (which is not defined in our Postgres version)
* The recording behavior of tablespace dependencies. Before in Yugabyte, `recordDependencyOnTablespace` was utilized on two main codepaths -- once in `index.c` on a call to `index_create` and once in `heap.c` on a call to `heap_create_with_catalog`. Instead, we simplify this to one codepath, being called in `heap.c` on `heap_create`, since any relation (table -- `heap_create_with_catalog.c`, index -- `index.c`, bootstrap catalogs -- `bootparse.y`) should have these dependencies recorded. As a result this is done on any Yugabyte relation or relation without storage where the tablespace is specified.
* Adding dependency recording for databases on their default tablespace. With `CREATE DATABASE`, the default tablespace is now tracked so it cannot be dropped prematurely, ensuring that the tablespace for a datba

Upstream Postgres commit was ebfe2dbd6b624e2a428e14b7ee9322cc096f63f7 with message:
```
When a tablespace is used in a partitioned relation (per commits
ca4103025dfe in pg12 for tables and 33e6c34c3267 in pg11 for indexes),
it is possible to drop the tablespace, potentially causing various
problems.  One such was reported in bug #16577, where a rewriting ALTER
TABLE causes a server crash.

Protect against this by using pg_shdepend to keep track of tablespaces
when used for relations that don't keep physical files; we now abort a
tablespace if we see that the tablespace is referenced from any
partitioned relations.

Backpatch this to 11, where this problem has been latent all along.  We
don't try to create pg_shdepend entries for existing partitioned
indexes/tables, but any ones that are modified going forward will be
protected.

Note slight behavior change: when trying to drop a tablespace that
contains both regular tables as well as partitioned ones, you'd
previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll
get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST.  Arguably, the latter is more
correct.

It is possible to add protecting pg_shdepend entries for existing
tables/indexes, by doing
  ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default;
  ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace;
for each partitioned table/index that is not in the database default
tablespace.  Because these partitioned objects do not have storage, no
file needs to be actually moved, so it shouldn't take more time than
what's required to acquire locks.

This query can be used to search for such relations:
SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/16577-881633a9f9894fd5@postgresql.org
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
```

Test Plan:
`./yb_build.sh --java-test org.yb.pgsql.TestPgRegressTablespaces`

The upstream tablespace regression test changes are not included in our test suite as the expected behavior is already tested. In particular, the result of the query `DROP TABLESPACE regress_tblspace;` under the `-- Fail, not empty` comment acts to confirm that dependencies are recorded in `pg_shdepend`. The core difference is that all tablespace dependencies are recorded in this table in Yugabyte, while Postgres uses tablespace directories for tracking dependencies (hence the sequence of drops in the SQL regression test).

Reviewers: alex, jason

Reviewed By: alex, jason

Subscribers: alex, dsrinivasan, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D13482
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 8, 2022
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature and removed kind/bug This issue is a bug labels Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue
Projects
Status: No status
Development

No branches or pull requests

2 participants