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

Add functions to enable/disable policies #6116

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+495 −1
Open

Add functions to enable/disable policies #6116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions .unreleased/PR_6116
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Implements: #6116 Add functions to enable/disable hypertable policies

91 changes: 90 additions & 1 deletion sql/policy_api.sql
View file
Open in desktop
Copy link
Contributor

@mkindahl mkindahl Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Missing similar functions for continuous aggregate policy.

Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,93 @@ CREATE OR REPLACE FUNCTION timescaledb_experimental.show_policies(
relation REGCLASS)
RETURNS SETOF JSONB
AS '@MODULE_PATHNAME@', 'ts_policies_show'
LANGUAGE C VOLATILE;
LANGUAGE C VOLATILE;

CREATE OR REPLACE FUNCTION _timescaledb_functions.set_policy_scheduled(hypertable REGCLASS, policy_type TEXT, scheduled BOOL)
RETURNS INTEGER
AS $$
WITH affected_policies AS (
SELECT @[email protected]_job(j.id, scheduled => set_policy_scheduled.scheduled)
FROM _timescaledb_config.bgw_job j
JOIN _timescaledb_catalog.hypertable h ON h.id = j.hypertable_id
WHERE j.proc_schema IN ('_timescaledb_internal', '_timescaledb_functions')
AND j.proc_name = set_policy_scheduled.policy_type
AND j.id >= 1000
AND scheduled <> set_policy_scheduled.scheduled
AND format('%I.%I', h.schema_name, h.table_name) = set_policy_scheduled.hypertable::text
)
SELECT count(*) FROM affected_policies;
$$
Comment on lines +133 to +148
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: instead of policy_type it might be better to just pass in the regproc ID and then match it with the proc_schema.proc_name cast to a regproc. Then it would be impossible to pass in a random text string.

Suggested change
CREATE OR REPLACE FUNCTION _timescaledb_functions.set_policy_scheduled(hypertable REGCLASS, policy_type TEXT, scheduled BOOL)
RETURNS INTEGER
AS $$
WITH affected_policies AS (
SELECT @[email protected]_job(j.id, scheduled => set_policy_scheduled.scheduled)
FROM _timescaledb_config.bgw_job j
JOIN _timescaledb_catalog.hypertable h ON h.id = j.hypertable_id
WHERE j.proc_schema IN ('_timescaledb_internal', '_timescaledb_functions')
AND j.proc_name = set_policy_scheduled.policy_type
AND j.id >= 1000
AND scheduled <> set_policy_scheduled.scheduled
AND format('%I.%I', h.schema_name, h.table_name) = set_policy_scheduled.hypertable::text
)
SELECT count(*) FROM affected_policies;
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;
CREATE OR REPLACE FUNCTION _timescaledb_functions.set_policy_scheduled(hypertable REGCLASS, policy_proc REGPROC, scheduled BOOL)
RETURNS INTEGER
AS $$
WITH affected_policies AS (
SELECT @[email protected]_job(j.id, scheduled => set_policy_scheduled.scheduled)
FROM _timescaledb_config.bgw_job j
JOIN _timescaledb_catalog.hypertable h ON h.id = j.hypertable_id
WHERE format('%I.%I', j.proc_schema, j.proc_name)::regproc = set_policy_scheduled.policy_proc
AND j.id >= 1000
AND scheduled <> set_policy_scheduled.scheduled
AND format('%I.%I', h.schema_name, h.table_name) = set_policy_scheduled.hypertable::text
)
SELECT count(*) FROM affected_policies;
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Consistency with the other names?

Suggested change
CREATE OR REPLACE FUNCTION _timescaledb_functions.set_all_policy_scheduled(hypertable REGCLASS, scheduled BOOL)
CREATE OR REPLACE FUNCTION _timescaledb_functions.set_all_policies_scheduled(hypertable REGCLASS, scheduled BOOL)

Copy link
Contributor

@gayyappan gayyappan Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This would not work with data tiering policies. We should treat that as a timescale policy too.
Also, the external API is disable_all_policies and enable_all_policies - as a user I would expect that all policies including any user defined policy would get disabled/enabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gayyappan Not sure I follow. Why would a different name of the above function not work with data tiering policies?

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably also need to check user's permissions before the user is allowed to enable/disable policies. A Should a user without adequate permissions on the hypertable be allowed to disable a policy? IIRC, we check the role's permissions before adding a policy.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mkindahl The proc_schema in the sql is limited to timescaledb_internal/timescaledb_functions. Not sure I follow. Why would a different name of the above function not work with data tiering policies?

Copy link
Contributor

Choose a reason for hiding this comment

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

@gayyappan Ah, so you were referring to the actual code for implementing the function, not the function name. It was a comment on my comment, so it wasn't entirely clear.

Copy link
Member Author

@JamesGuthrie JamesGuthrie Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This would not work with data tiering policies.

Correct. Should it? Data tiering policies reside in a separate extension. Should helper functions in the TimescaleDB extension wrap functions in the OSM extension?

as a user I would expect that all policies including any user defined policy would get disabled/enabled.

The function takes a hypertable as its argument, so it only disables policies which are attached to a hypertable. Maybe the name should change to {enable,disable}_all_policies_for_hypertable. From what I can tell, user-defined policies are never associated with a hypertable, so it wouldn't make sense to enable/disable user-defined policies from this function.

We probably also need to check user's permissions before the user is allowed to enable/disable policies.

All of these functions delegate to alter_job under the hood. Does that not perform the permission check that you mention? If not, which permissions would need to be checked?

Copy link
Contributor

@gayyappan gayyappan Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

While this is true from a code perspective, from a user perspective, this is 1 database running timescale defined policies - so it should just work out of the box. Correct. Should it? Data tiering policies reside in a separate extension. Should helper functions in the TimescaleDB extension wrap functions in the OSM extension?
Either we wrap OSM extension or add hooks so that the functionality can be extended by the OSM extension(in the same way that the timescale extension extends Postgres functionality).

Copy link
Contributor

Choose a reason for hiding this comment

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

All of these functions delegate to alter_job under the hood. Does that not perform the permission check that you mention? If not, which permissions would need to be checked?

That could very well be true. You could modify the test to show that a user/role without permissions on the hypertable cannot alter policies.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JamesGuthrie I'll add a follow up to get this to work with OSM extension as well. For this PR, we should verify that permissions are checked by the new apis and add a test case to confirm that.

CREATE OR REPLACE FUNCTION _timescaledb_functions.set_all_policy_scheduled(hypertable REGCLASS, scheduled BOOL)
RETURNS INTEGER
AS $$
WITH affected_policies AS (
SELECT @[email protected]_job(j.id, scheduled => set_all_policy_scheduled.scheduled)
FROM _timescaledb_config.bgw_job j
JOIN _timescaledb_catalog.hypertable h ON h.id = j.hypertable_id
WHERE j.proc_schema IN ('_timescaledb_internal', '_timescaledb_functions')
AND j.id >= 1000
AND scheduled <> set_all_policy_scheduled.scheduled
AND format('%I.%I', h.schema_name, h.table_name) = set_all_policy_scheduled.hypertable::text
)
SELECT count(*) FROM affected_policies;
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_all_policies(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_all_policy_scheduled(hypertable, false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_all_policies(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_all_policy_scheduled(hypertable, true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_compression_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_compression', false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_compression_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_compression', true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_reorder_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_reorder', false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_reorder_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_reorder', true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_retention_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_retention', false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE OR REPLACE FUNCTION @[email protected]_retention_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_retention', true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;
90 changes: 90 additions & 0 deletions sql/updates/latest-dev.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,93 @@ DROP TABLE _timescaledb_internal.tmp_chunk_seq_value;
GRANT SELECT ON _timescaledb_catalog.chunk_id_seq TO PUBLIC;
GRANT SELECT ON _timescaledb_catalog.chunk TO PUBLIC;
-- end recreate _timescaledb_catalog.chunk table --

CREATE FUNCTION _timescaledb_functions.set_policy_scheduled(hypertable REGCLASS, policy_type TEXT, scheduled BOOL)
RETURNS INTEGER
AS $$
WITH affected_policies AS (
SELECT @[email protected]_job(j.id, scheduled => set_policy_scheduled.scheduled)
FROM _timescaledb_config.bgw_job j
JOIN _timescaledb_catalog.hypertable h ON h.id = j.hypertable_id
WHERE j.proc_schema IN ('_timescaledb_internal', '_timescaledb_functions')
AND j.proc_name = set_policy_scheduled.policy_type
AND j.id >= 1000
AND scheduled <> set_policy_scheduled.scheduled
AND format('%I.%I', h.schema_name, h.table_name) = set_policy_scheduled.hypertable::text
)
SELECT count(*) FROM affected_policies;
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION _timescaledb_functions.set_all_policy_scheduled(hypertable REGCLASS, scheduled BOOL)
RETURNS INTEGER
AS $$
WITH affected_policies AS (
SELECT @[email protected]_job(j.id, scheduled => set_all_policy_scheduled.scheduled)
FROM _timescaledb_config.bgw_job j
JOIN _timescaledb_catalog.hypertable h ON h.id = j.hypertable_id
WHERE j.proc_schema IN ('_timescaledb_internal', '_timescaledb_functions')
AND j.id >= 1000
AND scheduled <> set_all_policy_scheduled.scheduled
AND format('%I.%I', h.schema_name, h.table_name) = set_all_policy_scheduled.hypertable::text
)
SELECT count(*) FROM affected_policies;
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_all_policies(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_all_policy_scheduled(hypertable, false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_all_policies(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_all_policy_scheduled(hypertable, true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_compression_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_compression', false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_compression_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_compression', true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_reorder_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_reorder', false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_reorder_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_reorder', true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_retention_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_retention', false);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

CREATE FUNCTION @[email protected]_retention_policy(hypertable REGCLASS)
RETURNS INTEGER
AS $$
SELECT _timescaledb_functions.set_policy_scheduled(hypertable, 'policy_retention', true);
$$
LANGUAGE SQL SET search_path TO pg_catalog, pg_temp;

11 changes: 11 additions & 0 deletions sql/updates/reverse-dev.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,14 @@ GRANT SELECT ON _timescaledb_catalog.chunk_id_seq TO PUBLIC;
GRANT SELECT ON _timescaledb_catalog.chunk TO PUBLIC;

-- end recreate _timescaledb_catalog.chunk table --

DROP FUNCTION _timescaledb_functions.set_policy_scheduled(hypertable REGCLASS, policy_type TEXT, scheduled BOOL);
DROP FUNCTION _timescaledb_functions.set_all_policy_scheduled(hypertable REGCLASS, scheduled BOOL);
DROP FUNCTION @[email protected]_all_policies(hypertable REGCLASS);
DROP FUNCTION @[email protected]_all_policies(hypertable REGCLASS);
DROP FUNCTION @[email protected]_compression_policy(hypertable REGCLASS);
DROP FUNCTION @[email protected]_compression_policy(hypertable REGCLASS);
DROP FUNCTION @[email protected]_reorder_policy(hypertable REGCLASS);
DROP FUNCTION @[email protected]_reorder_policy(hypertable REGCLASS);
DROP FUNCTION @[email protected]_retention_policy(hypertable REGCLASS);
DROP FUNCTION @[email protected]_retention_policy(hypertable REGCLASS);
Loading
Loading