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

Fix backfill.sql compression job rescheduling #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
50 changes: 39 additions & 11 deletions backfill.sql
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,23 @@ BEGIN
SELECT split_part(extversion, '.', 1)::INT INTO version FROM pg_catalog.pg_extension WHERE extname='timescaledb' LIMIT 1;

IF version = 1 THEN
SELECT job_id INTO compression_job_id FROM _timescaledb_config.bgw_policy_compress_chunks b WHERE b.hypertable_id = move_compression_job.hypertable_id;
SELECT job_id INTO compression_job_id FROM _timescaledb_config.bgw_policy_compress_chunks b WHERE b.hypertable_id = move_compression_job.hypertable_id;
Copy link

Choose a reason for hiding this comment

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

Please revert this whitespace change to keep the diff minimal:

Suggested change
SELECT job_id INTO compression_job_id FROM _timescaledb_config.bgw_policy_compress_chunks b WHERE b.hypertable_id = move_compression_job.hypertable_id;
SELECT job_id INTO compression_job_id FROM _timescaledb_config.bgw_policy_compress_chunks b WHERE b.hypertable_id = move_compression_job.hypertable_id;

ELSE
SELECT s.job_id INTO compression_job_id FROM timescaledb_information.jobs j
INNER JOIN timescaledb_information.job_stats s ON j.job_id = s.job_id
WHERE j.proc_name = 'policy_compression' AND s.hypertable_schema = schema_name AND s.hypertable_name = table_name;
SELECT j.job_id INTO compression_job_id FROM timescaledb_information.jobs j
WHERE j.proc_name = 'policy_compression' AND j.hypertable_schema = schema_name AND j.hypertable_name = table_name;

END IF;

IF compression_job_id IS NULL THEN
IF compression_job_id IS NULL THEN
Copy link

Choose a reason for hiding this comment

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

Ditto, please revert:

Suggested change
IF compression_job_id IS NULL THEN
IF compression_job_id IS NULL THEN

RAISE NOTICE 'No compression job found, no schedule to alter';
old_time = NULL::timestamptz;
ELSE
SELECT next_start INTO old_time FROM _timescaledb_internal.bgw_job_stat WHERE job_id = compression_job_id FOR UPDATE;

RAISE NOTICE 'Altering compression job schedule: id=%, old_time=%, new_time%', compression_job_id, old_time, new_time;
IF version = 1 THEN
PERFORM alter_job_schedule(compression_job_id, next_start=> new_time);
ELSE
ELSE
Copy link

Choose a reason for hiding this comment

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

Ditto, please revert:

Suggested change
ELSE
ELSE

PERFORM alter_job(compression_job_id, next_start=> new_time);
END IF;
END IF;
Expand Down Expand Up @@ -167,12 +169,32 @@ BEGIN

--And our time dimension, which is always the first dimension
SELECT d.* INTO STRICT dimension_row FROM _timescaledb_catalog.dimension d WHERE hypertable_id = hypertable_row.id ORDER BY id LIMIT 1 ;

-- Push the compression job out for some period of time so we don't end up compressing a decompressed chunk
-- Don't disable completely because at least then if we fail and fail to move it back things won't get completely weird
SELECT move_compression_job(hypertable_row.id, hypertable_row.schema_name, hypertable_row.table_name, now() + compression_job_push_interval) INTO old_compression_job_time;

COMMIT;

-- Push the compression job out for some period of time so we don't end up compressing a decompressed chunk
-- Don't disable completely because at least then if we fail and fail to move it back things won't get completely weird
LOOP
SELECT
move_compression_job(
hypertable_row.id,
hypertable_row.schema_name,
hypertable_row.table_name,
now() + compression_job_push_interval
)
INTO old_compression_job_time;
IF old_compression_job_time = '-infinity' :: timestamptz THEN
ROLLBACK;
RAISE NOTICE 'Compression job already running, sleeping...';
PERFORM pg_sleep(10);
ELSE
COMMIT;
RAISE NOTICE 'Compression job not already running, proceeding as normal...';
EXIT;
END IF;
END LOOP;
Comment on lines +175 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

If there are consecutive failures and you configure the max_retries option for the job the next_start will return NULL, so I guess you're not dealing with this case here. I mean the flow will continue to proceeding as normal... is this the expected behavior? Wondering if we'll not enter in a infinity loop here.

Copy link

Choose a reason for hiding this comment

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

Not sure, to be honest - it's a long time since we looked at this.

All I know is that we've used this successfully and not hit an infinite loop 🤷🏻



--Get the min and max times in timescale internal format from the source table, this will tell us which chunks we need to decompress
EXECUTE FORMAT($$SELECT _timescaledb_internal.time_to_internal(min(%1$I)) ,
_timescaledb_internal.time_to_internal(max(%1$I))
Expand Down Expand Up @@ -287,7 +309,13 @@ BEGIN
COMMIT;

--Move our job back to where it was
SELECT move_compression_job(hypertable_row.id, hypertable_row.schema_name, hypertable_row.table_name, old_compression_job_time) INTO old_compression_job_time;
SELECT
move_compression_job(
hypertable_row.id,
hypertable_row.schema_name,
hypertable_row.table_name,
old_compression_job_time
) INTO old_compression_job_time;
Comment on lines +312 to +318
Copy link
Contributor

Choose a reason for hiding this comment

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

We prefer to send separated PRs for code formatting because we can add it to .git-blame-ignore-revs

Copy link

Choose a reason for hiding this comment

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

@coiax Could you revert this formatting change as it's unrelated?

COMMIT;
END;

Expand Down