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.

Sign up for GitHub

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

Disable nonmaindb interface #7903

Closed
wants to merge 5 commits into from
Closed

Disable nonmaindb interface #7903

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9c5f6b
disable nonmaindb interface
eaydingol Feb 18, 2025
b09ff66
test fix
eaydingol Feb 18, 2025
38151ac
fix test
eaydingol Feb 18, 2025
5d7ff99
test fix
eaydingol Feb 18, 2025
90dde46
test fix
eaydingol Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions src/backend/distributed/shared_library_init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1834,16 +1834,6 @@ RegisterCitusConfigVariables(void)
GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE | GUC_UNIT_MS,
NULL, NULL, NULL);

DefineCustomStringVariable(
"citus.main_db",
gettext_noop("Which database is designated as the main_db"),
NULL,
&MainDb,
"",
PGC_POSTMASTER,
GUC_STANDARD,
NULL, NULL, NULL);

DefineCustomIntVariable(
"citus.max_adaptive_executor_pool_size",
gettext_noop("Sets the maximum number of connections per worker node used by "
Expand Down
8 changes: 0 additions & 8 deletions src/backend/distributed/sql/citus--12.1-1--12.2-1.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,21 @@
#include "udfs/citus_internal_database_command/12.2-1.sql"
#include "udfs/citus_add_rebalance_strategy/12.2-1.sql"

#include "udfs/start_management_transaction/12.2-1.sql"
#include "udfs/execute_command_on_remote_nodes_as_user/12.2-1.sql"
#include "udfs/mark_object_distributed/12.2-1.sql"
DROP FUNCTION pg_catalog.citus_unmark_object_distributed(oid, oid, int);
#include "udfs/citus_unmark_object_distributed/12.2-1.sql"
#include "udfs/commit_management_command_2pc/12.2-1.sql"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also delete these files altogether, and not just remove the include line here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal is to remove only the interface with minimal changes. I kept the files because their C implementations are still present in the codebase.

As you suggested, we can also remove the SQL and test files altogether. This way, we wouldn't need the name change and the additional schedule (from other comments).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the alternative #7905

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am leaning towards the alternative, especially for the .sql files. We don't keep .sql files with a Citus version in their name that we don't use.

I still think the changes are minimal if we remove the .sql files and the relevant tests as well. Wondering if codecov would complain though.


ALTER TABLE pg_catalog.pg_dist_transaction ADD COLUMN outer_xid xid8;

#include "udfs/citus_internal_acquire_citus_advisory_object_class_lock/12.2-1.sql"

GRANT USAGE ON SCHEMA citus_internal TO PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.commit_management_command_2pc FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.execute_command_on_remote_nodes_as_user FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.find_groupid_for_node FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.mark_object_distributed FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.pg_dist_node_trigger_func FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.pg_dist_rebalance_strategy_trigger_func FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.pg_dist_shard_placement_trigger_func FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.refresh_isolation_tester_prepared_statement FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.replace_isolation_tester_func FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.restore_isolation_tester_func FROM PUBLIC;
REVOKE ALL ON FUNCTION citus_internal.start_management_transaction FROM PUBLIC;

#include "udfs/citus_internal_add_colocation_metadata/12.2-1.sql"
#include "udfs/citus_internal_add_object_metadata/12.2-1.sql"
Expand Down
15 changes: 0 additions & 15 deletions src/backend/distributed/sql/downgrades/citus--12.2-1--12.1-1.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,9 @@ DROP FUNCTION citus_internal.acquire_citus_advisory_object_class_lock(int, cstri

#include "../udfs/citus_add_rebalance_strategy/10.1-1.sql"

DROP FUNCTION citus_internal.start_management_transaction(
outer_xid xid8
);

DROP FUNCTION citus_internal.execute_command_on_remote_nodes_as_user(
query text,
username text
);

DROP FUNCTION citus_internal.mark_object_distributed(
classId Oid, objectName text, objectId Oid, connectionUser text
);

DROP FUNCTION pg_catalog.citus_unmark_object_distributed(oid,oid,int,boolean);
#include "../udfs/citus_unmark_object_distributed/10.0-1.sql"

DROP FUNCTION citus_internal.commit_management_command_2pc();

ALTER TABLE pg_catalog.pg_dist_transaction DROP COLUMN outer_xid;
REVOKE USAGE ON SCHEMA citus_internal FROM PUBLIC;

Expand Down
0 ...itus_tests/test/test_maintenancedeamon.py → ...tus_tests/test/_test_maintenancedeamon.py
View file
Open in desktop
File renamed without changes.
0 .../citus_tests/test/test_other_databases.py → ...citus_tests/test/_test_other_databases.py
View file
Open in desktop
File renamed without changes.
2 changes: 0 additions & 2 deletions src/test/regress/expected/citus_internal_access.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
CREATE USER nonsuperuser CREATEROLE;
SET ROLE nonsuperuser;
--- The non-superuser role should not be able to access citus_internal functions
SELECT citus_internal.commit_management_command_2pc();
ERROR: permission denied for function commit_management_command_2pc
SELECT citus_internal.replace_isolation_tester_func();
ERROR: permission denied for function replace_isolation_tester_func
RESET ROLE;
Expand Down
6 changes: 0 additions & 6 deletions src/test/regress/expected/create_drop_database_propagation_pg15.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,5 @@ SELECT * FROM public.check_database_on_all_nodes('test_locale_provider') ORDER B
worker node (remote) | {"database_properties": {"datacl": null, "datname": "test_locale_provider", "datctype": "C", "encoding": "UTF8", "datcollate": "C", "tablespace": "pg_default", "daticurules": null, "datallowconn": true, "datconnlimit": -1, "daticulocale": null, "datistemplate": false, "database_owner": "postgres", "datcollversion": null, "datlocprovider": "c"}, "pg_dist_object_record_for_db_exists": true, "stale_pg_dist_object_record_for_a_db_exists": false}
(3 rows)

\c test_locale_provider - - :worker_2_port
set citus.enable_create_database_propagation to on;
create database unsupported_option_from_non_main_db with oid = 12345;
ERROR: CREATE DATABASE option "oid" is not supported
\c regression - - :master_port
set citus.enable_create_database_propagation to on;
drop database test_locale_provider;
\c - - - :master_port
6 changes: 1 addition & 5 deletions src/test/regress/expected/multi_extension.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1431,28 +1431,24 @@ SELECT * FROM multi_extension.print_extension_changes();
| function citus_internal.add_shard_metadata(regclass,bigint,"char",text,text) void
| function citus_internal.add_tenant_schema(oid,integer) void
| function citus_internal.adjust_local_clock_to_remote(cluster_clock) void
| function citus_internal.commit_management_command_2pc() void
| function citus_internal.database_command(text) void
| function citus_internal.delete_colocation_metadata(integer) void
| function citus_internal.delete_partition_metadata(regclass) void
| function citus_internal.delete_placement_metadata(bigint) void
| function citus_internal.delete_shard_metadata(bigint) void
| function citus_internal.delete_tenant_schema(oid) void
| function citus_internal.execute_command_on_remote_nodes_as_user(text,text) void
| function citus_internal.global_blocked_processes() SETOF record
| function citus_internal.is_replication_origin_tracking_active() boolean
| function citus_internal.local_blocked_processes() SETOF record
| function citus_internal.mark_node_not_synced(integer,integer) void
| function citus_internal.mark_object_distributed(oid,text,oid,text) void
| function citus_internal.start_management_transaction(xid8) void
| function citus_internal.start_replication_origin_tracking() void
| function citus_internal.stop_replication_origin_tracking() void
| function citus_internal.unregister_tenant_schema_globally(oid,text) void
| function citus_internal.update_none_dist_table_metadata(oid,"char",bigint,boolean) void
| function citus_internal.update_placement_metadata(bigint,integer,integer) void
| function citus_internal.update_relation_colocation(oid,integer) void
| function citus_unmark_object_distributed(oid,oid,integer,boolean) void
(30 rows)
(26 rows)

DROP TABLE multi_extension.prev_objects, multi_extension.extension_diff;
-- show running version
Expand Down
6 changes: 1 addition & 5 deletions src/test/regress/expected/upgrade_list_citus_objects.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,27 +64,23 @@ ORDER BY 1;
function citus_internal.add_shard_metadata(regclass,bigint,"char",text,text)
function citus_internal.add_tenant_schema(oid,integer)
function citus_internal.adjust_local_clock_to_remote(cluster_clock)
function citus_internal.commit_management_command_2pc()
function citus_internal.database_command(text)
function citus_internal.delete_colocation_metadata(integer)
function citus_internal.delete_partition_metadata(regclass)
function citus_internal.delete_placement_metadata(bigint)
function citus_internal.delete_shard_metadata(bigint)
function citus_internal.delete_tenant_schema(oid)
function citus_internal.execute_command_on_remote_nodes_as_user(text,text)
function citus_internal.find_groupid_for_node(text,integer)
function citus_internal.global_blocked_processes()
function citus_internal.is_replication_origin_tracking_active()
function citus_internal.local_blocked_processes()
function citus_internal.mark_node_not_synced(integer,integer)
function citus_internal.mark_object_distributed(oid,text,oid,text)
function citus_internal.pg_dist_node_trigger_func()
function citus_internal.pg_dist_rebalance_strategy_trigger_func()
function citus_internal.pg_dist_shard_placement_trigger_func()
function citus_internal.refresh_isolation_tester_prepared_statement()
function citus_internal.replace_isolation_tester_func()
function citus_internal.restore_isolation_tester_func()
function citus_internal.start_management_transaction(xid8)
function citus_internal.start_replication_origin_tracking()
function citus_internal.stop_replication_origin_tracking()
function citus_internal.unregister_tenant_schema_globally(oid,text)
Expand Down Expand Up @@ -371,5 +367,5 @@ ORDER BY 1;
view citus_stat_tenants_local
view pg_dist_shard_placement
view time_partitions
(361 rows)
(357 rows)

1 change: 0 additions & 1 deletion src/test/regress/failure_schedule
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ test: failure_multi_row_insert
test: failure_mx_metadata_sync
test: failure_mx_metadata_sync_multi_trans
test: failure_connection_establishment
test: failure_non_main_db_2pc
test: failure_create_database

# this test syncs metadata to the workers
Expand Down
3 changes: 1 addition & 2 deletions src/test/regress/multi_1_schedule
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ test: create_drop_database_propagation_pg15
test: create_drop_database_propagation_pg16
test: comment_on_database
test: comment_on_role
test: metadata_sync_from_non_maindb
# don't parallelize single_shard_table_udfs to make sure colocation ids are sequential
test: single_shard_table_udfs
test: schema_based_sharding
Expand All @@ -58,7 +57,7 @@ test: multi_metadata_attributes

test: multi_read_from_secondaries

test: grant_on_database_propagation grant_on_database_propagation_from_non_maindb
test: grant_on_database_propagation
test: alter_database_propagation

test: citus_shards
Expand Down
1 change: 0 additions & 1 deletion src/test/regress/multi_schedule
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ test: object_propagation_debug
test: undistribute_table
test: run_command_on_all_nodes
test: background_task_queue_monitor
test: other_databases grant_role_from_non_maindb role_operations_from_non_maindb seclabel_non_maindb
test: citus_internal_access
test: function_with_case_when

Expand Down
4 changes: 4 additions & 0 deletions src/test/regress/non_maindb_schedule
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
test: failure_non_main_db_2pc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this schedule runnable?
If not, maybe we don't need to introduce this schedule.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actually, it includes the removed tests. "check_all_tests_are_run" complains without it.

test: metadata_sync_from_non_maindb
test: grant_on_database_propagation_from_non_maindb
test: other_databases grant_role_from_non_maindb role_operations_from_non_maindb seclabel_non_maindb
1 change: 0 additions & 1 deletion src/test/regress/pg_regress_multi.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,6 @@ sub generate_hba
push(@pgOptions, "citus.enable_change_data_capture=on");
push(@pgOptions, "citus.stat_tenants_limit = 2");
push(@pgOptions, "citus.stat_tenants_track = 'ALL'");
push(@pgOptions, "citus.main_db = 'regression'");
push(@pgOptions, "citus.superuser = 'postgres'");

# Some tests look at shards in pg_class, make sure we can usually see them:
Expand Down
1 change: 0 additions & 1 deletion src/test/regress/sql/citus_internal_access.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ CREATE USER nonsuperuser CREATEROLE;

SET ROLE nonsuperuser;
--- The non-superuser role should not be able to access citus_internal functions
SELECT citus_internal.commit_management_command_2pc();
SELECT citus_internal.replace_isolation_tester_func();

RESET ROLE;
Expand Down
8 changes: 0 additions & 8 deletions src/test/regress/sql/create_drop_database_propagation_pg15.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ CREATE DATABASE test_locale_provider

SELECT * FROM public.check_database_on_all_nodes('test_locale_provider') ORDER BY node_type;

\c test_locale_provider - - :worker_2_port

set citus.enable_create_database_propagation to on;
create database unsupported_option_from_non_main_db with oid = 12345;

\c regression - - :master_port

set citus.enable_create_database_propagation to on;
drop database test_locale_provider;

\c - - - :master_port
Loading