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

bulk-io: CREATE TABLE can cause IMPORT to fail when run concurrently on a multi-region database #70987

Closed
ajstorm opened this issue Oct 1, 2021 · 1 comment · Fixed by #71016
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ajstorm
Copy link
Collaborator

ajstorm commented Oct 1, 2021

IMPORT requires the type descriptor to be stable over the course of the operation. This is an issue for multi-region databases, as some CREATE TABLE operations (REGIONAL BY TABLE in <region> and REGIONAL BY ROW) will cause the type to be updated to add a back reference. The end result is that if one of these CREATE TABLE operations occurs concurrently with an IMPORT operation, the IMPORT will fail with:

type descriptor 53 has a different modification time than what was saved during import planning; unsafe to import since the type has changed during the course of the import

One possible solution to this specific problem is for IMPORT to cache not just the modification time, but the actual type descriptor. Then, at the end of the IMPORT job, it could determine if something meaningful changed in the type (say, the allowable values) as opposed to something superficial (the back references).

@ajstorm ajstorm added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-disaster-recovery T-disaster-recovery labels Oct 1, 2021
@ajstorm
Copy link
Collaborator Author

ajstorm commented Oct 1, 2021

Note that this is a very specific symptom of the problem described in #69706.

ajwerner added a commit to ajwerner/cockroach that referenced this issue Oct 1, 2021
In cockroachdb#69674 we enabled importing of tables which used UDTs. We added the caveat
that these types must not change during the import. In cockroachdb#70987, @ajstorm
uncovered that adding new usages of the type cause an illegal change. This is
a particularly painful limitation as all regional by row tables will use the
enum type of the database. That makes the limitation of import much more
extreme than just precluding renames or modifications of enums or their
members.

To fix this limitation, we permit changes to the referencing set which
occur during the import. We also permit changes to privileges as they
won't impact the correctness of the import.

Relates to cockroachdb#69706
Fixes cockroachdb#70987

Release note (enterprise change): Fixed a limitation of IMPORT for tables
using user-defined types whereby any change to the set of tables or views
which reference the type or any changes to privileges on the type during
the IMPORT would lead to failure. Now new references to the type or GRANT
or REVOKE operations performed while the IMPORT is ongoing will not cause
failure.
ajwerner added a commit to ajwerner/cockroach that referenced this issue Oct 1, 2021
In cockroachdb#69674 we enabled importing of tables which used UDTs. We added the caveat
that these types must not change during the import. In cockroachdb#70987, @ajstorm
uncovered that adding new usages of the type cause an illegal change. This is
a particularly painful limitation as all regional by row tables will use the
enum type of the database. That makes the limitation of import much more
extreme than just precluding renames or modifications of enums or their
members.

To fix this limitation, we permit changes to the referencing set which
occur during the import. We also permit changes to privileges as they
won't impact the correctness of the import.

Relates to cockroachdb#69706
Fixes cockroachdb#70987

Release note (enterprise change): Fixed a limitation of IMPORT for tables
using user-defined types whereby any change to the set of tables or views
which reference the type or any changes to privileges on the type during
the IMPORT would lead to failure. Now new references to the type or GRANT
or REVOKE operations performed while the IMPORT is ongoing will not cause
failure.
ajwerner added a commit to ajwerner/cockroach that referenced this issue Oct 1, 2021
In cockroachdb#69674 we enabled importing of tables which used UDTs. We added the caveat
that these types must not change during the import. In cockroachdb#70987, @ajstorm
uncovered that adding new usages of the type cause an illegal change. This is
a particularly painful limitation as all regional by row tables will use the
enum type of the database. That makes the limitation of import much more
extreme than just precluding renames or modifications of enums or their
members.

To fix this limitation, we permit changes to the referencing set which
occur during the import. We also permit changes to privileges as they
won't impact the correctness of the import.

Relates to cockroachdb#69706
Fixes cockroachdb#70987

Release note (enterprise change): Fixed a limitation of IMPORT for tables
using user-defined types whereby any change to the set of tables or views
which reference the type or any changes to privileges on the type during
the IMPORT would lead to failure. Now new references to the type or GRANT
or REVOKE operations performed while the IMPORT is ongoing will not cause
failure.
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Oct 4, 2021
craig bot pushed a commit that referenced this issue Oct 12, 2021
71016: importccl: allow changes to referencing set or privileges on UDT r=ajwerner a=ajwerner

In #69674 we enabled importing of tables which used UDTs. We added the caveat
that these types must not change during the import. In #70987, @ajstorm
uncovered that adding new usages of the type cause an illegal change. This is
a particularly painful limitation as all regional by row tables will use the
enum type of the database. That makes the limitation of import much more
extreme than just precluding renames or modifications of enums or their
members.

To fix this limitation, we permit changes to the referencing set which
occur during the import. We also permit changes to privileges as they
won't impact the correctness of the import.

Relates to #69706
Fixes #70987

Release note (enterprise change): Fixed a limitation of IMPORT for tables
using user-defined types whereby any change to the set of tables or views
which reference the type or any changes to privileges on the type during
the IMPORT would lead to failure. Now new references to the type or GRANT
or REVOKE operations performed while the IMPORT is ongoing will not cause
failure.

71121: colexec: take advantage of partial ordering in the hash aggregator r=rharding6373 a=rharding6373

This change consists of two commits. The first adds test coverage for 
aggregation with partial ordering of group columns, including adding a new
microbenchmark. The second adds support for partial ordering in the hash 
aggregator.

Results from both the new benchmark, HashAggregatorPartialOrder, and existing
hash aggregator benchmarks show improved performance for the partial order 
case for lower limit values, and no performance degradation for the unordered 
case: 
https://gist.github.com/rharding6373/09eface5ed49a00aac540a8ea8304662

This change is fairly large, so I will follow this PR with an update to the optimizer 
cost model to take advantage of partial ordering.

71420: roachtest/hibernate: fix failing test r=rafiss a=otan

Resolves #71312

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: rharding6373 <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
@craig craig bot closed this as completed in 2e99b40 Oct 12, 2021
blathers-crl bot pushed a commit that referenced this issue Oct 12, 2021
In #69674 we enabled importing of tables which used UDTs. We added the caveat
that these types must not change during the import. In #70987, @ajstorm
uncovered that adding new usages of the type cause an illegal change. This is
a particularly painful limitation as all regional by row tables will use the
enum type of the database. That makes the limitation of import much more
extreme than just precluding renames or modifications of enums or their
members.

To fix this limitation, we permit changes to the referencing set which
occur during the import. We also permit changes to privileges as they
won't impact the correctness of the import.

Relates to #69706
Fixes #70987

Release note (enterprise change): Fixed a limitation of IMPORT for tables
using user-defined types whereby any change to the set of tables or views
which reference the type or any changes to privileges on the type during
the IMPORT would lead to failure. Now new references to the type or GRANT
or REVOKE operations performed while the IMPORT is ongoing will not cause
failure.
@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 labels May 10, 2023
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. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants