Consider adding a check for unlogged / temp tables in citus_rebalance_start() too

[ivyazmitinov]

Description

If there is UNLOGGED table present, logical rebalancing fails with

ERROR: cannot add relation "test_table_102008" to publication
DETAIL: This operation is not supported for unlogged tables.
CONTEXT: while executing command on worker-1-1:5432
Citus Background Task Queue Executor: postgres/postgres for (1/1)

Steps to reproduce

• Initialize a cluster with at least one node
• Create a distributed UNLOGGED table

  CREATE UNLOGGED TABLE test_table
   (
       user_id text PRIMARY KEY
   );
   SELECT create_distributed_table('test_table', 'user_id');

• Add new to the cluster
• Start rebalancing

  SELECT citus_rebalance_start(rebalance_strategy := 'by_shard_count');

• Observe error in citus_rebalance_status()

[onurctirtir]

Since Postgres doesn't support replicating temp / unlogged tables, non-blocking rebalancer also doesn't support this. The only workaround I can recommend would be un-colocating the distributed unlogged table from its colocation group to continue benefiting from non-blocking rebalancer for the other distributed tables in that colocation group (if any), before starting the shard-rebalancer.

So, although the user experience is not great, this is a known limitation.

[ivyazmitinov]

@onurctirtir, we went in a bit a different direction: make tables LOGGED temporarily, it may be a viable option for the Citus rebalancer as well.

In any case, if it is a known limitation, it would be nice to have a meaningful error message at the rebalancing start, similar to tables without primary keys

[onurctirtir]

In any case, if it is a known limitation, it would be nice to have a meaningful error message at the rebalancing start, similar to tables without primary keys

+1 to that. Actually, the underlying shard-rebalancer infra performs such checks at the very early stages but the error is reported to the rebalancer monitor, not to the client side directly; and I agree that mimicking such quick checks at the very beginning of citus_rebalance_start() would make it much more explicit.