[YSQL] Columns with default values may be initialized to null instead
#2021
Assignees
Milestone
Comments
yugabyte-ci
added
community/request
|
Notes:
|
frozenspider
added
kind/bug
frozenspider
changed the title
foo timestamp default NOW() column may not be initializedfoo timestamp default NOW() column may not be initialized
|
I've changed PG Datum
now(PG_FUNCTION_ARGS)
{
elog(LOG, " === Calling NOW() ===");
TimestampTz txnStartTs = GetCurrentTransactionStartTimestamp();
Datum result = TimestampTzGetDatum(txnStartTs);
elog(LOG, " === Start TS: %ld, result: %ld ===", txnStartTs, result);
return result;
}When using When tested with Jepsen, let's take a look at this log excerpt (click to expand)We can search for the first alex@alex-debian-jessie:~/code/jepsen/yugabyte$ grep -iR '=== Start TS' store/latest/ | cut -d' ' -f2 | sort | head -n 1
15:37:43.851Note that first |
|
Interesting thing: we're actually writing tombstone instead of NOW |
|
@m-iancu made an assumption about the cause:
This appears to be correct, the following Java test reliably reproduces this: |
|
Should be fixed as part of #1404 |
aphyr
changed the title
foo timestamp default NOW() column may not be initializednull instead
|
I've expanded the test which uncovered this issue into a workload which specifically demonstrates the issue and reports its presence. Note that this problem affects any default value, not just timestamps or |
24 tasks
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
|
This has been resolved and is no longer reproducible. This was confirmed by verifying the following:
Note that in order to be safe, the testing was done using 100 iterations of the Jepsen test (using Jepsen commit |
|
Heyyyy congats!!! :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In YB 1.3.1.0, on Debian, it looks like columns with a default value of
NOW()may, occasionally, be initialized toNULLinstead. With a schema likeAnd a workload comprised of inserts (
INSERT INTO foo (v) VALUES (?)), and reads (SELECT k, v FROM foo ORDER BY k), it appears that sometimes,kmay wind upNULL, rather than being a timestamp. This log line shows a series of [k, v] pairs from a read; notice that it contains one row (13) with a timestamp, and one row (9) with null. 20190812T154729.000-0400.zipOther tables may contain no null entries, or many nulls. These null values appear stable over multiple seconds. To reproduce, try Jepsen
e324b33, and runThe checker doesn't verify this property, so the test failing or succeeding isn't meaningful. Instead, grep the logs for a line with a table with nil entries.
The text was updated successfully, but these errors were encountered: