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

Router planning for modify queries fails to detect shard replication factor mismatch #6779

Closed
onurctirtir opened this issue Mar 20, 2023 · 1 comment · Fixed by #6909
Closed
Assignees
Labels
Milestone

Comments

@onurctirtir
Copy link
Member

onurctirtir commented Mar 20, 2023

And that causes some modify queries to fail very late because we think that it's router planneble.

SET citus.shard_replication_factor TO 1;

create table single_shard_dist(a int, b int);
SELECT create_distributed_table('single_shard_dist', 'a', shard_count=>1);
SET citus.shard_replication_factor TO 2;

CREATE TABLE distributed_table(a int, b int);
SELECT create_distributed_table('distributed_table', 'a', shard_count=>4);
WITH cte AS (
    DELETE FROM distributed_table WHERE a = 1 RETURNING *
)
SELECT * FROM single_shard_dist WHERE b IN (SELECT b FROM cte);
ERROR:  relation "public.single_shard_dist_102008" does not exist
CONTEXT:  while executing command on localhost:9702

Making router planner colocation aware (#692) would automatically fix this too.

@onurctirtir onurctirtir changed the title Router planner fails to detect shard replication factor mismatch Router planning for modify queries fails to detect shard replication factor mismatch Mar 20, 2023
onurctirtir added a commit that referenced this issue Mar 28, 2023
…ed tables via router planner (#6793)

Today we allow planning the queries that reference non-colocated tables
if the shards that query targets are placed on the same node. However,
this may not be the case, e.g., after rebalancing shards because it's
not guaranteed to have those shards on the same node anymore.
This commit adds citus.enable_non_colocated_router_query_pushdown GUC
that can be used to disallow  planning such queries via router planner,
when it's set to false. Note that the default value for this GUC will be
"true" for 11.3, but we will alter it to "false" on 12.0 to not
introduce
a breaking change in a minor release.

Closes #692.

Even more, allowing such queries to go through router planner also
causes
generating an incorrect plan for the DML queries that reference
distributed
tables that are sharded based on different replication factor settings.
For
this reason, #6779 can be closed after altering the default value for
this
GUC to "false", hence not now.

DESCRIPTION: Adds `citus.enable_non_colocated_router_query_pushdown` GUC
to ensure generating a consistent distributed plan for the queries that
reference non-colocated distributed tables (when set to "false", the
default is "true").
@onurctirtir onurctirtir added this to the 12.0 Release milestone Mar 28, 2023
@onurctirtir onurctirtir self-assigned this Mar 28, 2023
@onurctirtir
Copy link
Member Author

#6793 (comment):

Even more, allowing such queries to go through router planner also causes
generating an incorrect plan for the DML queries that reference distributed
tables that are sharded based on different replication factor settings. For
this reason, #6779 can be closed after altering the default value for this
GUC to "false", hence not now.

onurctirtir added a commit that referenced this issue May 15, 2023
…6909)

Fixes #6779.

DESCRIPTION: Disables citus.enable_non_colocated_router_query_pushdown
GUC by default to ensure generating a consistent distributed plan for
the queries that reference non-colocated distributed tables

We already have tests for the cases where this GUC is disabled,
so I'm not adding any more tests in this PR.

Also make multi_insert_select_window idempotent.

Related to: #6793
emelsimsek pushed a commit that referenced this issue May 24, 2023
…6909)

Fixes #6779.

DESCRIPTION: Disables citus.enable_non_colocated_router_query_pushdown
GUC by default to ensure generating a consistent distributed plan for
the queries that reference non-colocated distributed tables

We already have tests for the cases where this GUC is disabled,
so I'm not adding any more tests in this PR.

Also make multi_insert_select_window idempotent.

Related to: #6793
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant