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

Jump to bottom

Support running on PostgreSQL 15 #664

Merged
merged 4 commits into from Jan 3, 2023
Merged

Support running on PostgreSQL 15 #664

merged 4 commits into from Jan 3, 2023

Conversation

syvb
Copy link
Member

@syvb syvb commented Dec 19, 2022 •
edited

Fixes #648.

  • Fix all doctests that call setseed
  • Fix TimescaleDB crash in CI
  • Fix random function on pg12/pg13
  • Don't try to run upgrade tester on releases without PG15 binaries (currently this is all releases)

Things to fix in release-build-scripts (these don't block this PR):

@syvb
Copy link
Member Author

syvb commented Dec 19, 2022 •
edited

The CI for this PR fails on PG15:

running 15 test files
Starting stateless tests
Starting docs/asap.md
Starting docs/stats_agg.md
Finished docs/asap.md
error dropping DB connection closed
Starting docs/client.md
Finished docs/stats_agg.md
thread '<unnamed>' panicked at 'could not connect to postgres: Error { kind: Db, cause: Some(DbError { severity: "FATAL", parsed_severity: Some(Fatal), code: SqlState(E57P03), message: "the database system is in recovery mode", detail: None, hint: None, position: None, where_: None, schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("postmaster.c"), line: Some(2403), routine: Some("ProcessStartupPacket") }) }', tools/sql-doctester/src/runner.rs:79:18

I got a similar failure the first time I ran the doc-tests locally, the second and third times I got strange results.

@syvb syvb mentioned this pull request Dec 19, 2022
Closed
@syvb syvb force-pushed the sv/docker-pg15 branch 11 times, most recently from f301a38 to cf2040a Compare December 23, 2022 19:30
@syvb syvb force-pushed the sv/docker-pg15 branch 3 times, most recently from 352470f to cbf5064 Compare January 3, 2023 15:00
@syvb
Copy link
Member Author

syvb commented Jan 3, 2023

The CI passes when run with the updated Docker image: https://github.com/timescale/timescaledb-toolkit/actions/runs/3830536476

epgts
epgts reviewed Jan 3, 2023
View reviewed changes
docker/ci/setup.sh Outdated Show resolved Hide resolved
epgts
epgts reviewed Jan 3, 2023
View reviewed changes
tools/dependencies.sh Outdated Show resolved Hide resolved
@syvb syvb changed the title Add PG15 support to Docker image Support running on PostgreSQL 15 Jan 3, 2023
@syvb syvb mentioned this pull request Jan 3, 2023
Merged
@syvb syvb marked this pull request as ready for review January 3, 2023 18:15
syvb
syvb commented Jan 3, 2023
View reviewed changes
.github/workflows/patch_build.yml
cargo run --manifest-path ./tools/sql-doctester/Cargo.toml -- -h localhost -s "CREATE EXTENSION timescaledb_toolkit; SET SESSION TIMEZONE TO 'UTC'" -p 5432 docs
cargo run --manifest-path ./tools/sql-doctester/Cargo.toml -- -h localhost -p 5432 docs
Copy link
Member Author

Choose a reason for hiding this comment

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

This CI job has bitrotted and always fails to run (it's only run when certain files are modified). The patch Docker image should probably be completely removed in a future PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

motion seconded

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked @JLockerman if I could get rid of it the first time I tripped over it (PR #442). At the time, he thought it was still useful, though I can't find any written record of our conversation about it.

I think it's pretty clear we don't need it now...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this thing only ever had a very specific use–getting a version of the current TimescaleDB docker image with the latest dev version of Toolkit installed–and was only ever useful as an internal debugging tool. It was always rather brittle, and if we're not finding it useful anymore we might as well delete it.

bors bot added a commit that referenced this pull request Jan 3, 2023
@bors @syvb
Merge #670
c2b7f75 
670: Add PG15 support to Docker image r=Smittyvb a=Smittyvb

This PR has just the Docker image changes from #664. ([CI run](https://github.com/timescale/release-build-scripts/actions/runs/3831893814))

Co-authored-by: Smitty <smitty@timescale.com>
@syvb
Copy link
Member Author

syvb commented Jan 3, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 3, 2023

Build succeeded:

@bors bors bot merged commit 4df0b89 into main Jan 3, 2023
@bors bors bot deleted the sv/docker-pg15 branch January 3, 2023 21:24
@syvb syvb mentioned this pull request Jan 4, 2023
Merged
bors bot added a commit that referenced this pull request Jan 4, 2023
@bors @syvb
Merge #674
145cf0a 
674: Remove bitrotted patch image r=Smittyvb a=Smittyvb

The `patch` Docker image hasn't been successfully built for 8 months, and the job to build it always fails due to an authentication error. Even if it got past that issue, it would still fail since it's still using pgx 0.2.

This PR removes the image and the CI job that builds it.

(originally noted at #664 (comment))

Co-authored-by: Smitty <smitty@timescale.com>
bors bot added a commit that referenced this pull request Jan 4, 2023
@bors @syvb
Merge #674
2f2d91c 
674: Remove bitrotted patch image r=Smittyvb a=Smittyvb

The `patch` Docker image hasn't been successfully built for 8 months, and the job to build it always fails due to an authentication error. Even if it got past that issue, it would still fail since it's still using pgx 0.2.

This PR removes the image and the CI job that builds it.

(originally noted at #664 (comment))

Co-authored-by: Smitty <smitty@timescale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JLockerman JLockerman JLockerman left review comments

@WireBaron WireBaron WireBaron approved these changes

@epgts epgts epgts approved these changes

@thatzopoulos thatzopoulos Awaiting requested review from thatzopoulos

@rtwalker rtwalker Awaiting requested review from rtwalker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Run doctests and update tests on PostgreSQL 15
5 participants
@syvb @rtwalker @JLockerman @WireBaron @epgts