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] Rows inserted by DDL are not visible to the encompassing repeatable read transaction #3110

Open
mbautin opened this issue Dec 7, 2019 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@mbautin
Copy link
Collaborator

mbautin commented Dec 7, 2019

Jira Link: DB-4822
This is an issue with running DDL statements inside their separate DocDB-level transactions as implemented by #3108.

E.g. from yb_pg_rowsecurity.sql:

--
-- Collation support
-- Running in serializable mode to pick up rows created within the separate DDL transaction.
--
BEGIN;
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
GRANT SELECT ON coll_t TO regress_rls_alice;
SELECT (string_to_array(polqual, ':'))[7] AS inputcollid FROM pg_policy WHERE polrelid = 'coll_t'::regclass;
SET SESSION AUTHORIZATION regress_rls_alice;
SELECT * FROM coll_t;
ROLLBACK;

Here, we were required to set isolation level to SERIALIZABLE, or SELECT * FROM coll_t; would see no rows, as its read point was selected before the table coll_t was created, and the rows inserted as part of the creation of that table are not visible to that read point.

@mbautin mbautin self-assigned this Dec 7, 2019
mbautin added a commit that referenced this issue Dec 12, 2019
…ction

Summary:
This diff makes a step towards enabling transactional DDL in YugabyteDB
by switching to transactional execution of multiple YSQL system catalog
read/write operations that happen as part of the same DDL operation.

We are making YSQL system catalog tables transactional at the DocDB
level, backed by the usual transaction status table that resides on
tablet servers. This introduces an additional dependency of the master
on tablet servers, so that accessing YSQL system catalog tables in the
common case now requires tablet servers to be up and running and the
transaction status table to exist, because there could be provisional
records whose status is determined by a status tablet living on tablet
servers. This new dependency could be removed later by making the system
catalog tablet a transaction status tablet in addition to everything
else it already stores.

Previously, we used to create the transaction status table whenever the
first request to create a transactional table (including any YSQL table)
was received. Now, the transaction status table is also required by
operations such as creating a role in YSQL, or anything that reads or
changes the state of YSQL system catalog tables. Therefore, if YSQL is
enabled, we now create the transaction status table automatically on the
master leader right after the number of tablet servers equal to the
cluster's replication factor have registered.

After we've added DocDB transactional capabilities to YSQL system
catalog tables, there are multiple possible choices in how we could wrap
formerly non-transactional DDL operations that access the YSQL system
catalog into transactions.  Ideally, we would want to allow arbitrary
user-controlled transactions to include DDL statements that could be
committed/rolled back similarly to DML statements. That would require us
to commit and roll back the corresponding Yugabyte-level operations
(creating namespaces, tablets, and tablets in the Yugabyte system
catalog, which is also stored in the master but is distinct from the
YSQL system catalog) along with YSQL system catalog changes. This more
complete feature is tracked at #3109 and is not part of this diff.

This diff's main goal is an incremental improvement over the existing
state of completely non-transactional YSQL system catalog changes that
would allow a reader to e.g. see a state of a half-done CREATE TABLE
statement. With this diff, all DDL statements are wrapped in a separate
DocDB-level transaction that is distinct from the regular DocDB-level
transaction matching the user's YSQL transaction. One such separate
DDL-only transaction is created per each top-level DML statement,
including any internal sub-statements that it issues. This should solve
the problem of leaving the effects of a half-done CREATE TABLE statement
in the system catalog, and the problem of observing results of an
incomplete CREATE TABLE statement (e.g. before it sets the default value
of a column, as in #2021). However, this could produce some anomalies,
e.g. #3110, that will be addressed as we implement full transactional
DDL support (see #3109).  Even though the example given in #3110
involves row-level security, this diff does not negatively affect row
level security specificially. Any CREATE TABLE statement executed as
part of a user-controlled transaction will now create and commit rows as
part of a separate DDL-only tranasction, and if the enclosing
user-controlled transaction is running at REPEATABLE READ isolation,
further statements in the user-controlled transaction won't see those
rows. They would still see the newly created table, because internal
read operations on system catalog tables are always done outside of any
transaction.  YugabyteDB's SERIALIZABLE transactions always read the
most recent state of the data and lock it against concurrent changes, so
switching the isolation level to SERIALIZABLE is sufficient to fix
the yb_pg_rowsecurity test.

These separate DDL-only transactions are executed at the REPEATABLE READ
isolation level by default, but can be set to serializable by the
setting the new `--ysql_enable_manual_sys_table_txn_ctl` flag.

We are adding a new flag to table properties, is_ysql_catalog_table. To
initialize this flag for existing clusters, we loop over all YSQL system
catalog tables and replicate the necessary changes to table entities in
the YugabyteDB system catalog, as well as updates to the system catalog
tablet metadata. For the tablet metadata change we use the existing
add_table field in ChangeMetadataRequestPB. This was originally intended
for use with new tables only, but would only produce an error message on
cluster upgrades in release builds (due to a DCHECK), and effectively
replace the old metadata protobuf with the new one.

Test Plan:
Jenkins

Upgrade test:

~/code/yugabyte-db-old/bin/yb-ctl create

Load the Northwind database as described at
https://docs.yugabyte.com/latest/sample-data/northwind/

~/code/yugabyte-db-old/bin/yb-ctl stop

~/code/yugabyte-db-new/bin/yb-ctl start

Look for these messages in the master log:

I1212 22:13:36.832412 2963256 sys_catalog_initialization.cc:312] Made 655 YSQL sys catalog tables transactional
I1212 22:13:36.832451 2963256 sys_catalog_initialization.cc:315] Marking YSQL system catalog as transactional in YSQL catalog config

Verify that the Northwind data is intact.

Repeat the above steps with `--replication_factor=3` as well.

Reviewers: mihnea, sergei, neha

Reviewed By: sergei, neha

Subscribers: bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D7156
@mbautin mbautin changed the title [YSQL] Rows inserted by DDL are not visible to the encompassing repeatable read [YSQL] Rows inserted by DDL are not visible to the encompassing repeatable read transaction Dec 13, 2019
@rthallamko3 rthallamko3 added the area/ysql Yugabyte SQL (YSQL) label Jan 3, 2023
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jan 3, 2023
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/bug This issue is a bug priority/medium Medium priority issue
Projects
Status: No status
Development

No branches or pull requests

3 participants