Skip to content

fix(pg): do not batch with copyFrom - #17402

Merged
janisz merged 1 commit into
masterfrom
fix_pgx_copy_from
Nov 7, 2025
Merged

fix(pg): do not batch with copyFrom#17402
janisz merged 1 commit into
masterfrom
fix_pgx_copy_from

Conversation

@janisz

@janisz janisz commented Oct 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Refactored the generated copyFrom functions to use pgx.CopyFromFunc with an iterator pattern instead of pre-building row slices. This improves memory efficiency for bulk insert operations.

I run benchmark from

                          │  master1.txt   │               new1.txt                │
                          │ max_heap_bytes │ max_heap_bytes  vs base               │
Many/upsert_128_alerts-8      1.826M ± 24%     1.291M ± 34%  -29.34% (p=0.002 n=6)
Many/upsert_256_alerts-8      3.176M ±  2%     2.096M ±  1%  -33.99% (p=0.002 n=6)
Many/upsert_512_alerts-8      5.413M ± 14%     2.915M ±  4%  -46.14% (p=0.002 n=6)
Many/upsert_1024_alerts-8     8.834M ±  6%     3.287M ± 10%  -62.80% (p=0.002 n=6)
Many/upsert_2048_alerts-8    14.597M ± 16%     3.564M ± 59%  -75.59% (p=0.002 n=6)
Many/upsert_4096_alerts-8    19.687M ± 15%     4.050M ± 15%  -79.43% (p=0.002 n=6)
Many/upsert_8192_alerts-8    38.786M ±  4%     4.660M ± 13%  -87.98% (p=0.002 n=6)
geomean                       4.628M           2.509M        -45.80%

                          │   master1.txt    │                new1.txt                 │
                          │ max_heap_objects │ max_heap_objects  vs base               │
Many/upsert_128_alerts-8        1.826M ± 24%       1.291M ± 34%  -29.34% (p=0.002 n=6)
Many/upsert_256_alerts-8        3.176M ±  2%       2.096M ±  1%  -33.99% (p=0.002 n=6)
Many/upsert_512_alerts-8        5.413M ± 14%       2.915M ±  4%  -46.14% (p=0.002 n=6)
Many/upsert_1024_alerts-8       8.834M ±  6%       3.287M ± 10%  -62.80% (p=0.002 n=6)
Many/upsert_2048_alerts-8      14.597M ± 16%       3.564M ± 59%  -75.59% (p=0.002 n=6)
Many/upsert_4096_alerts-8      19.687M ± 15%       4.050M ± 15%  -79.43% (p=0.002 n=6)
Many/upsert_8192_alerts-8      38.786M ±  4%       4.660M ± 13%  -87.98% (p=0.002 n=6)
geomean                         4.628M             2.509M        -45.80%

Refs:

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

change me!

@janisz
janisz requested review from a team as code owners October 21, 2025 08:25
@rhacs-bot

rhacs-bot commented Oct 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Images are ready for the commit at 2202443.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.10.x-200-g22024434f5.

@codecov

codecov Bot commented Oct 21, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.14097% with 1052 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.70%. Comparing base (4a68181) to head (2202443).
⚠️ Report is 74 commits behind head on master.

Files with missing lines Patch % Lines
...mplianceoperator/v2/report/store/postgres/store.go 0.00% 48 Missing ⚠️
...ral/cve/image/v2/datastore/store/postgres/store.go 0.00% 42 Missing ⚠️
...able-bindings/testgraphtables/testparent1/store.go 0.00% 41 Missing ⚠️
...magecomponent/v2/datastore/store/postgres/store.go 0.00% 35 Missing ⚠️
...reports/snapshot/datastore/store/postgres/store.go 0.00% 35 Missing ⚠️
...table-bindings/testgraphtables/testchild2/store.go 0.00% 28 Missing ⚠️
...indings/testgraphtables/testg2grandchild1/store.go 0.00% 28 Missing ⚠️
...-bindings/testgraphtables/testgrandchild1/store.go 0.00% 28 Missing ⚠️
central/policycategoryedge/store/postgres/store.go 0.00% 27 Missing ⚠️
...ble-bindings/testgraphtables/testchild1p4/store.go 0.00% 27 Missing ⚠️
... and 84 more
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #17402      +/-   ##
==========================================
- Coverage   48.72%   48.70%   -0.03%     
==========================================
  Files        2728     2728              
  Lines      203127   202964     -163     
==========================================
- Hits        98980    98851     -129     
+ Misses      96399    96366      -33     
+ Partials     7748     7747       -1
Flag Coverage Δ
go-unit-tests 48.70% <66.14%> (-0.03%) ⬇️

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.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dashrews78
dashrews78 reviewed Oct 21, 2025
View reviewed changes
Comment thread tools/generate-helpers/pg-table-bindings/store.go.tpl
@dashrews78

dashrews78 commented Oct 21, 2025

Copy link
Copy Markdown
Collaborator

One more ask before I approve. I assume we have a benchmark somewhere that exercises this path. Can we get a comparison of the before and after in the description?

rhybrillou
rhybrillou reviewed Oct 21, 2025
View reviewed changes

@rhybrillou rhybrillou left a comment

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.

Please split the batch fix, the test change and the store change into separated PRs.

Comment thread tools/generate-helpers/pg-table-bindings/store.go.tpl
rhybrillou
rhybrillou reviewed Oct 22, 2025
View reviewed changes
Comment thread central/alert/datastore/internal/store/postgres/bench_test.go
rhybrillou
rhybrillou reviewed Oct 24, 2025
View reviewed changes
Comment thread tools/generate-helpers/pg-table-bindings/store.go.tpl Outdated
rhybrillou
rhybrillou reviewed Oct 24, 2025
View reviewed changes
Comment thread tools/generate-helpers/pg-table-bindings/store.go.tpl
rhybrillou
rhybrillou reviewed Oct 24, 2025
View reviewed changes
Comment thread tools/generate-helpers/pg-table-bindings/store.go.tpl
@janisz

janisz commented Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

I extracted test changes to: #17562

@janisz
janisz force-pushed the fix_pgx_copy_from branch from 8b77eb7 to 130068d Compare October 30, 2025 10:48
@janisz
fix(pg): do not batch with copyFrom
2202443 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz
janisz force-pushed the fix_pgx_copy_from branch from 130068d to 2202443 Compare October 30, 2025 11:20
@janisz
janisz merged commit 5262b7a into master Nov 7, 2025
93 checks passed
@janisz
janisz deleted the fix_pgx_copy_from branch November 7, 2025 12:07
ajheflin pushed a commit that referenced this pull request Nov 7, 2025
@janisz @ajheflin
fix(pg): do not batch with copyFrom (#17402)
f42cdce 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhybrillou rhybrillou rhybrillou approved these changes
@dashrews78 dashrews78 dashrews78 approved these changes

Assignees

No one assigned

Labels

area/central area/postgres

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@janisz @rhacs-bot @dashrews78 @rhybrillou