Skip to content

Conversation

joseph-sentry
Copy link
Contributor

@joseph-sentry joseph-sentry commented Jul 16, 2025

Fixes CCMRG-1370

we're enabling Timescale's hypercore feature for the TA testrun hypertable and
the testrun summary and testrun branch summary CAs. We're doing this for the
space savings, since we're not really doing point queries on the testruns table.

We're segmenting by repoid and testid because we will almost always filter on
repo_id if we're running queries against any TA table, and I'm choosing to also
segment by test_id since we're likely to group by the test_id and it'd be likely
for multiple different tests to all run around the same time, which I suspect
would reduce the effectiveness of the compression for the computed_name and
test_id columns, since we'd have to store a bunch of different values instead
of just one or two for a given chunk.

Copy link

linear bot commented Jul 16, 2025

Copy link

codecov bot commented Jul 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.28%. Comparing base (45e74cc) to head (f3fef84).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #327   +/-   ##
=======================================
  Coverage   94.28%   94.28%           
=======================================
  Files        1247     1248    +1     
  Lines       45911    45916    +5     
  Branches     1455     1455           
=======================================
+ Hits        43289    43294    +5     
  Misses       2317     2317           
  Partials      305      305           
Flag Coverage Δ
sharedintegration 40.00% <0.00%> (-0.02%) ⬇️
sharedunit 88.72% <100.00%> (+<0.01%) ⬆️
workerintegration 61.55% <ø> (ø)
workerunit 90.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link

codecov-notifications bot commented Jul 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link

codspeed-hq bot commented Jul 16, 2025

CodSpeed Performance Report

Merging #327 will not alter performance

Comparing joseph/hypercore (f3fef84) with main (45e74cc)

Summary

✅ 9 untouched benchmarks

@rohan-at-sentry
Copy link

@sentry review

This comment has been minimized.

Comment on lines 40 to +41
timescale:
image: timescale/timescaledb-ha:pg14-latest
image: timescale/timescaledb:latest-pg14

This comment was marked as outdated.

Comment on lines 146 to +147
timescale:
image: timescale/timescaledb-ha:pg14-latest
image: timescale/timescaledb:latest-pg14

This comment was marked as outdated.

Comment on lines 63 to 64
FROM ta_timeseries_testrun
WHERE branch IN ('main', 'master', 'develop')

This comment was marked as outdated.

Comment on lines 89 to 95
"""
SELECT add_continuous_aggregate_policy(
'ta_timeseries_aggregate_hourly',
start_offset => '2 hours',
end_offset => NULL,
schedule_interval => INTERVAL '1 hour'
);

This comment was marked as outdated.

Comment on lines 13 to 18
RiskyRunSQL(
"""
CALL refresh_continuous_aggregate(
'ta_timeseries_aggregate_hourly',
NOW() - INTERVAL '60 days',
NULL

This comment was marked as outdated.

Comment on lines 13 to 19
"""
ALTER TABLE ta_timeseries_testrun SET (
timescaledb.enable_columnstore,
timescaledb.segmentby = 'repo_id, test_id',
timescaledb.orderby = 'timestamp DESC'
);
""",

This comment was marked as outdated.

we're enabling Timescale's hypercore feature for the TA testrun hypertable and
the testrun summary and testrun branch summary CAs. We're doing this for the
space savings, since we're not really doing point queries on the testruns table.

We're segmenting by repoid and branch because we will almost always filter on
repo_id if we're running queries against any TA table, and I'm choosing to also
segment by branch because we'll be filtering by branch in some of the CAs and
when we're reading raw data from the table in the API.

For the orderby section I'm choosing to order by testsuite, classname, name then
timestamp because i want to have similar tests grouped together in chunks so
we can make the most of the compression and also because we're grouping by those
fields in the computations for the CAs.

the old image is 2 years old and is using timescale 2.10.2. We want to use the
hypercore API so we have to have at least 2.18.0, so I'm updating the image
we're using so CI works.
@joseph-sentry
Copy link
Contributor Author

@sentry review

@joseph-sentry joseph-sentry marked this pull request as ready for review July 30, 2025 14:17
@joseph-sentry joseph-sentry requested a review from a team as a code owner July 30, 2025 14:17
Comment on lines 40 to +41
timescale:
image: timescale/timescaledb-ha:pg14-latest
image: timescale/timescaledb:latest-pg14

This comment was marked as outdated.

Comment on lines 146 to +147
timescale:
image: timescale/timescaledb-ha:pg14-latest
image: timescale/timescaledb:latest-pg14

This comment was marked as outdated.

Comment on lines +22 to +27
RiskyRunSQL(
"""
CALL add_columnstore_policy('ta_timeseries_testrun', INTERVAL '7 days');
""",
reverse_sql="CALL remove_columnstore_policy('ta_timeseries_testrun');",
),

This comment was marked as outdated.

Comment on lines +5 to +6

class Migration(migrations.Migration):

This comment was marked as outdated.

Comment on lines +14 to +26
ALTER TABLE ta_timeseries_testrun SET (
timescaledb.enable_columnstore,
timescaledb.segmentby = 'repo_id, branch',
timescaledb.orderby = 'testsuite, classname, name, timestamp DESC'
);
""",
reverse_sql="ALTER TABLE ta_timeseries_testrun SET (timescaledb.enable_columnstore = false);",
),
RiskyRunSQL(
"""
CALL add_columnstore_policy('ta_timeseries_testrun', INTERVAL '7 days');
""",
reverse_sql="CALL remove_columnstore_policy('ta_timeseries_testrun');",
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential bug: Migration may fail due to TimescaleDB version incompatibility with PostgreSQL 14 in the new Docker image, causing deployment failure.
  • Description: The migration attempts to enable TimescaleDB hypercore columnstore features and use functions like add_columnstore_policy() and parameters like timescaledb.enable_columnstore. These features require TimescaleDB 2.18.0+. However, the pull request changes the Docker image to timescale/timescaledb:latest-pg14. Recent TimescaleDB versions (2.20.0+) no longer officially support PostgreSQL 14. This incompatibility means the required TimescaleDB functions or parameters may not be available in the deployed environment, leading to ProgrammingError or similar database errors during migration execution. This will cause deployment failures and service outages.
  • Suggested fix: Verify the exact TimescaleDB version in timescale/timescaledb:latest-pg14 and ensure it supports both PostgreSQL 14 and the required hypercore columnstore features. If not, update the Docker image to a compatible TimescaleDB version that supports PostgreSQL 15+ or adjust the migration to features compatible with the existing environment.
    severity: 0.9, confidence: 0.75

Did we get this right? 👍 / 👎 to inform future reviews.

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

Successfully merging this pull request may close these issues.

2 participants