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

system.public.table_statistics table definition has discrepancy after upgrading from a old version #77979

Closed
Xiang-Gu opened this issue Mar 16, 2022 · 2 comments · Fixed by #78302
Assignees
Labels
branch-master Failures and bugs on the master branch. branch-release-22.1 Used to mark GA and release blockers, technical advisories, and bugs for 22.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@Xiang-Gu
Copy link
Contributor

Xiang-Gu commented Mar 16, 2022

When we upgrade CRDB binary version running in nodes in a cluster, migration logic will be triggered to make sure tables in system database are "consistent" (i.e. same definition) as that of a newly bootstrapped cluster.

A recent roachtest showed there is discrepancy in the system.public.table_statistics table. Namely, the actual return of CREATE TABLE after upgrading is`

	CREATE TABLE public.table_statistics (	
	"tableID" INT8 NOT NULL,	
	"statisticID" INT8 NOT NULL DEFAULT unique_rowid(),	
	name STRING NULL,	
	"columnIDs" INT8[] NOT NULL,	
	"createdAt" TIMESTAMP NOT NULL DEFAULT now():::TIMESTAMP,	
	"rowCount" INT8 NOT NULL,	
	"distinctCount" INT8 NOT NULL,	
	"nullCount" INT8 NOT NULL,	
	histogram BYTES NULL,	
	"avgSize" INT8 NOT NULL DEFAULT 0:::INT8,	
	CONSTRAINT "primary" PRIMARY KEY ("tableID" ASC, "statisticID" ASC),	
	FAMILY "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_histogram" ("tableID", "statisticID", name, "columnIDs", "createdAt", "rowCount", "distinctCount", "nullCount", histogram),	
	FAMILY "fam_10_avgSize" ("avgSize")	
);

while the expected output (as hard-coded in systemschema) is

CREATE TABLE public.table_statistics (
	"tableID" INT8 NOT NULL,
	"statisticID" INT8 NOT NULL DEFAULT unique_rowid(),
	name STRING NULL,
	"columnIDs" INT8[] NOT NULL,
	"createdAt" TIMESTAMP NOT NULL DEFAULT now():::TIMESTAMP,
	"rowCount" INT8 NOT NULL,
	"distinctCount" INT8 NOT NULL,
	"nullCount" INT8 NOT NULL,
	"avgSize" INT8 NOT NULL DEFAULT 0:::INT8,
	histogram BYTES NULL,
	CONSTRAINT "primary" PRIMARY KEY ("tableID" ASC, "statisticID" ASC),
	FAMILY "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_avgSize_histogram" ("tableID", "statisticID", name, "columnIDs", "createdAt", "rowCount", "distinctCount", "nullCount", "avgSize", histogram)
);

Two things need to be done to close this issue:

  1. Change systemschema to make sure the ordering of the last two columns are corrected;
  2. Investigate and make sure the newly added column (avgSize) does not have a family of its own. Instead, it should be placed into the existing family.

Jira issue: CRDB-13880

@Xiang-Gu Xiang-Gu added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker labels Mar 16, 2022
@Xiang-Gu Xiang-Gu self-assigned this Mar 16, 2022
@blathers-crl
Copy link

blathers-crl bot commented Mar 16, 2022

Hi @Xiang-Gu, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Mar 16, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 16, 2022
@postamar postamar added branch-release-22.1 Used to mark GA and release blockers, technical advisories, and bugs for 22.1 branch-master Failures and bugs on the master branch. labels Mar 21, 2022
@rytaft rytaft assigned rharding6373 and unassigned Xiang-Gu Mar 22, 2022
@rytaft
Copy link
Collaborator

rytaft commented Mar 22, 2022

@rharding6373 I'm giving you this one since it relates to avgSize

rharding6373 added a commit to rharding6373/cockroach that referenced this issue Mar 23, 2022
Before this change, the new `system.table_statistics` column `avgSize`
introduced in version 22.1.12 was appended to the end of the table
during migration, but the system schema had the new column in a
different order. The column was also not added to the existing column
family containing all table columns during migration.

This change fixes both the system schema and the migration commands so
that the column ordering is the same and the new column is added to the
existing column family. Unfortunately, this means that the existing
column family name is unable to be updated to include the column.

Fixes: cockroachdb#77979

Release note (sql): Fixes a migration discrepancy from 22.1.10 to
22.1.12 where the column ordering differed between the system schema and
the table after migration. Additionally, it ensures that during
migration the column added as part of the migration is added to an
existing column family.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Mar 23, 2022
Before this change, the new `system.table_statistics` column `avgSize`
introduced in version 21.2.12 was appended to the end of the table
during migration, but the system schema had the new column in a
different order. The column was also not added to the existing column
family containing all table columns during migration.

This change fixes both the system schema and the migration commands so
that the column ordering is the same and the new column is added to the
existing column family. Unfortunately, this means that the existing
column family name is unable to be updated to include the column.

Fixes: cockroachdb#77979

Release note (sql): Fixes a migration discrepancy from 21.2.10 to
21.2.12 where the column ordering differed between the system schema and
the table after migration. Additionally, it ensures that during
migration the column added as part of the migration is added to an
existing column family.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue Mar 23, 2022
Before this change, the new `system.table_statistics` column `avgSize`
introduced in version 21.2.12 was appended to the end of the table
during migration, but the system schema had the new column in a
different order. The column was also not added to the existing column
family containing all table columns during migration.

This change fixes both the system schema and the migration commands so
that the column ordering is the same and the new column is added to the
existing column family. Unfortunately, this means that the existing
column family name is unable to be updated to include the column.

Fixes: cockroachdb#77979

Release note: None
craig bot pushed a commit that referenced this issue Mar 25, 2022
78014: streamingccl: make producer job exit smoothly after ingestion cutover r=gh-casper a=gh-casper

Previously producer job will time out and fail automatically after
ingestion cutover as consumer stops sending heartbeats.
This is not a good UX experience since stream replication is successful
but showed up failed.

This PR adds a new crdb builtin "crdb_internal.complete_replication_stream"
to let consumer send signal to source cluster that ingestion happens.

Closes: #76954
Release justification: Cat 4.
Release note: none.

78302: sql: fix migration with new system.table_statistics column r=rharding6373 a=rharding6373

Before this change, the new `system.table_statistics` column `avgSize`
introduced in version 22.1.12 was appended to the end of the table
during migration, but the system schema had the new column in a
different order. The column was also not added to the existing column
family containing all table columns during migration.

This change fixes both the system schema and the migration commands so
that the column ordering is the same and the new column is added to the
existing column family. Unfortunately, this means that the existing
column family name is unable to be updated to include the column.

Fixes: #77979

Release justification: Fixes a schema migration bug in the
table_statistics table.

Release note: None

78410: changefeedccl: remove tenant timestamp protection gates r=samiskin a=samiskin

Now that protected timestamps function in tenants in 22.1 the pts gates in
changefeeds can be removed.

Resolves #76936

Release justification: low risk change turning off now-unneeded gates
Release note (enterprise change): changefeeds can now protect targets
from gc on user tenants

78445: colexec: use Bytes.Copy instead of Get and Set in most places r=yuzefovich a=yuzefovich

**coldata: fix the usage of Bytes.Copy in CopyWithReorderedSource**

This was the intention but wasn't working because the call happens
inside a separate template.

Release note: None

**colexec: use Bytes.Copy instead of Get and Set in most places**

This commit audits our code for the usage of `Bytes.Get` followed by
`Bytes.Set` pattern and replaces those with `Bytes.Copy` (which is
faster for inlined values) in non-test code.

Release note: None

78456: roachtest: wait for ranges to replicate before filling disk r=tbg a=nicktrav

Currently, the `disk-full` roachtest creates a cluster and immediately
places a ballast file on one node, which causes it to crash. If this
node is the only replica for a range containing a system table, when the
node crashes due to a full disk certain system queries may not complete.
This results in the test being unable to make forward progress, as the
one dead node prevents a system query from completing, and this query
prevents the node from being restarted.

Wait for all ranges to have at least two replicas before placing the
ballast file on the one node.

Touches #78337, #78270.

Release note: None.

78468: sql: return an error when partition spans has no healthy instances r=rharding6373 a=rharding6373

If there are no SQL instances available for planning,
partitionSpansTenant in the DistSQL planner will panic. This PR fixes
the issue so that it instead returns an error if there are no instances
available.

Fixes: #77590

Release justification: Fixes a bug in DistSQL that can cause a panic for
non-system tenants.

Release note: None

Co-authored-by: Casper <[email protected]>
Co-authored-by: rharding6373 <[email protected]>
Co-authored-by: Shiranka Miskin <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
@craig craig bot closed this as completed in 40ffa99 Mar 25, 2022
blathers-crl bot pushed a commit that referenced this issue Mar 25, 2022
Before this change, the new `system.table_statistics` column `avgSize`
introduced in version 21.2.12 was appended to the end of the table
during migration, but the system schema had the new column in a
different order. The column was also not added to the existing column
family containing all table columns during migration.

This change fixes both the system schema and the migration commands so
that the column ordering is the same and the new column is added to the
existing column family. Unfortunately, this means that the existing
column family name is unable to be updated to include the column.

Fixes: #77979

Release justification: Fixes a schema migration bug in the
table_statistics table.

Release note: None
fqazi pushed a commit to fqazi/cockroach that referenced this issue Apr 4, 2022
Before this change, the new `system.table_statistics` column `avgSize`
introduced in version 21.2.12 was appended to the end of the table
during migration, but the system schema had the new column in a
different order. The column was also not added to the existing column
family containing all table columns during migration.

This change fixes both the system schema and the migration commands so
that the column ordering is the same and the new column is added to the
existing column family. Unfortunately, this means that the existing
column family name is unable to be updated to include the column.

Fixes: cockroachdb#77979

Release justification: Fixes a schema migration bug in the
table_statistics table.

Release note: None
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead T-sql-queries SQL Queries Team labels May 10, 2023
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. branch-release-22.1 Used to mark GA and release blockers, technical advisories, and bugs for 22.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants