Skip to content

Add Timescale DB with Hypertable and Retention support #1517

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

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

m4rcs
Copy link
Contributor

@m4rcs m4rcs commented Jun 11, 2024

With this pull I want to add support of Timescale DB to blocky.

It uses the postgres connection (queryLog.target=postgres://...) of GORM to write data, but uses hypertabels to store time-series data and uses Timescales-native retention with queryLog.type=timescale.

Copy link
Owner

@0xERR0R 0xERR0R left a comment

Choose a reason for hiding this comment

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

I didn't test it, but it looks good so far for me 👍 . We do have some e2e tests with testcontainers (real database as docker container) for query log. Maybe you could create e2e test for timescale DB? If not, I can also do it.

@0xERR0R 0xERR0R added this to the v0.25 milestone Jun 25, 2024
@0xERR0R 0xERR0R added the 🔨 enhancement New feature or request label Jun 25, 2024
@m4rcs m4rcs force-pushed the feature/timescaledb branch from 2aa3f67 to a05f4db Compare July 5, 2024 19:31
@m4rcs
Copy link
Contributor Author

m4rcs commented Jul 9, 2024

I added some e2e tests. Is this ok?

@m4rcs m4rcs force-pushed the feature/timescaledb branch 2 times, most recently from 5f5d410 to 300c385 Compare July 9, 2024 19:23
Copy link
Owner

@0xERR0R 0xERR0R left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you for your work!

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 59.57447% with 19 lines in your changes missing coverage. Please review.

Project coverage is 93.92%. Comparing base (fe84ab8) to head (d9b6a5b).
Report is 97 commits behind head on main.

Files Patch % Lines
querylog/database_writer.go 30.43% 16 Missing ⚠️
resolver/query_logging_resolver.go 87.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1517      +/-   ##
==========================================
+ Coverage   93.88%   93.92%   +0.04%     
==========================================
  Files          78       79       +1     
  Lines        6361     5056    -1305     
==========================================
- Hits         5972     4749    -1223     
+ Misses        300      217      -83     
- Partials       89       90       +1     

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

@0xERR0R
Copy link
Owner

0xERR0R commented Jul 10, 2024

Please fix the linter error to make build green again, then we can merge

@m4rcs m4rcs force-pushed the feature/timescaledb branch from 300c385 to d9b6a5b Compare July 12, 2024 09:55
@m4rcs
Copy link
Contributor Author

m4rcs commented Jul 12, 2024

The linter error is fixed. I'll try to increase coverage later :)

@0xERR0R
Copy link
Owner

0xERR0R commented Jul 16, 2024

The linter error is fixed. I'll try to increase coverage later :)

Should we merge it now or do you want to add some tests with this PR?

@m4rcs
Copy link
Contributor Author

m4rcs commented Jul 16, 2024

You can merge now. I'll open another PR for the tests. Thanks.

@0xERR0R 0xERR0R merged commit 6259e13 into 0xERR0R:main Jul 17, 2024
10 of 11 checks passed
@m4rcs m4rcs deleted the feature/timescaledb branch July 23, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants