Skip to content

feat(postgres): Optimize storage - #175

Merged
devsjc merged 6 commits into
mainfrom
devsjc/optimize-storage
Jun 17, 2026
Merged

feat(postgres): Optimize storage#175
devsjc merged 6 commits into
mainfrom
devsjc/optimize-storage

Conversation

@devsjc

@devsjc devsjc commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Modifies the schema of the predicted generation values table for optimal storage. Reduces table size by about 40%.
It has been tested thoroughly locally, as well as partially in dev. All looks to work well.

Contribution Checklist

  • Have you followed the Open Climate Fix Contribution Guidelines?
  • Have you referenced the Issue this PR addresses, where applicable?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added a summary of the changes?
  • Have you written new tests for your changes, where applicable?
  • Have you successfully run make lint with your changes locally?
  • Have you successfully run make test with your changes locally?

Warning

PRs may be closed if all the above boxes are not checked.

Closes #171

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown

Benchmark Results

Benchmark results
?   	github.com/openclimatefix/data-platform/cmd	[no test files]
?   	github.com/openclimatefix/data-platform/internal/gen/ocf/dp	[no test files]
?   	github.com/openclimatefix/data-platform/internal/interceptors	[no test files]
PASS
ok  	github.com/openclimatefix/data-platform/internal/server/dummy	0.005s
{"level":"debug","time":"2026-06-17T09:47:52Z","message":"Completed migrations"}
goos: linux
goarch: amd64
pkg: github.com/openclimatefix/data-platform/internal/server/postgres
cpu: AMD EPYC 7763 64-Core Processor                
BenchmarkPostgresClient/small/GetForecastAsTimeseries-4         	      49	  23437365 ns/op
BenchmarkPostgresClient/small/GetForecastAtTimestamp-4          	     193	   6124355 ns/op
BenchmarkPostgresClient/small/GetObservationsAsTimeseries-4     	     934	   1221710 ns/op
BenchmarkPostgresClient/small/CreateForecast-4                  	     108	  10700664 ns/op
BenchmarkPostgresClient/small/StreamForecastData-4              	      21	  52124910 ns/op
PASS
ok  	github.com/openclimatefix/data-platform/internal/server/postgres	66.896s
?   	github.com/openclimatefix/data-platform/internal/server/postgres/gen	[no test files]
Benchmark vs base branch
goos: linux
goarch: amd64
pkg: github.com/openclimatefix/data-platform/internal/server/postgres
cpu: AMD EPYC 7763 64-Core Processor                
                                                   │ bench-devsjc-optimize-storage.txt │
                                                   │              sec/op               │
PostgresClient/small/GetForecastAsTimeseries-4                            23.44m ± ∞ ¹
PostgresClient/small/GetForecastAtTimestamp-4                             6.124m ± ∞ ¹
PostgresClient/small/GetObservationsAsTimeseries-4                        1.222m ± ∞ ¹
PostgresClient/small/CreateForecast-4                                     10.70m ± ∞ ¹
PostgresClient/small/StreamForecastData-4                                 52.12m ± ∞ ¹
geomean                                                                   9.956m
¹ need >= 6 samples for confidence interval at level 0.95

cpu: AMD EPYC 9V74 80-Core Processor                
                                                   │ bench-main.txt │
                                                   │     sec/op     │
PostgresClient/small/GetForecastAsTimeseries-4         18.67m ± ∞ ¹
PostgresClient/small/GetForecastAtTimestamp-4          4.202m ± ∞ ¹
PostgresClient/small/GetObservationsAsTimeseries-4     1.205m ± ∞ ¹
PostgresClient/small/CreateForecast-4                  11.74m ± ∞ ¹
PostgresClient/small/StreamForecastData-4              102.4m ± ∞ ¹
geomean                                                10.26m
¹ need >= 6 samples for confidence interval at level 0.95

devsjc added 3 commits June 16, 2026 16:49
This was a field that as of yet wasn't being used. In order to make sure
this very large table doesn't take up more storage space than it needs
to, I'm removing this dynamic column. Any value-related metadata can
live in the forecast metadata, I'm pretty comfident.
@peterdudfield

Copy link
Copy Markdown
Contributor

Just so you know (this PR might still be the right thing to do), that we want to add 5 extras plevels fairly soon, but probably only for National. I assume there is still an option to add this to other_stats_fractions.

Also, with this change (and deployment), do we have to update the forecast apps to save to these new columns?

pg.horizon_mins,
pg.p10_sip,
pg.p50_sip,
pg.other_stats_fractions,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we keep this in, so we can easily add new statistics without having to change the data-platform

@devsjc

devsjc commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

With this current change, nothing has to be updated. P10s and P90s are mapped from the existing other_stats argument in the protobuf, which hasn't been modified. However, new ones won't be saved, as it stands.

As I mentioned in the issue write up though, it's really easy to add new columns for P levels. Sure it's a change in the data platform, but it isn't a difficult one. And having 10-20 smallint columns that are mostly null is absolutely fine, and as it turns out, much better for storage than one dynamic column. (In fact, storing 10 P levels in this regard would still take up less than half the storage space of storing just 2 in the jsonb column as before. I think, with storage being a concern, this is the right way to go about things. (It also lends itself much better to columnar compression, if we use that in future!)

What are the new levels expecting to be added? I can bundle them in with this change for ease.

@peterdudfield

Copy link
Copy Markdown
Contributor

With this current change, nothing has to be updated. P10s and P90s are mapped from the existing other_stats argument in the protobuf, which hasn't been modified. However, new ones won't be saved, as it stands.

As I mentioned in the issue write up though, it's really easy to add new columns for P levels. Sure it's a change in the data platform, but it isn't a difficult one. And having 10-20 smallint columns that are mostly null is absolutely fine, and as it turns out, much better for storage than one dynamic column. (In fact, storing 10 P levels in this regard would still take up less than half the storage space of storing just 2 in the jsonb column as before. I think, with storage being a concern, this is the right way to go about things. (It also lends itself much better to columnar compression, if we use that in future!)

What are the new levels expecting to be added? I can bundle them in with this change for ease.

Thanks, makes sense

Sorry it could be p2, p10, p25, (and above), but its not really sorted yet. So i would probably not include it in here

peterdudfield
peterdudfield previously approved these changes Jun 17, 2026
@devsjc devsjc changed the title perf(postgres): Optimize table feat(postgres): Optimize storage Jun 17, 2026
@devsjc
devsjc merged commit 25281a1 into main Jun 17, 2026
4 checks passed
@devsjc
devsjc deleted the devsjc/optimize-storage branch June 17, 2026 09:55
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.

[DE] Reduce size of database

2 participants