ROX-28939: Create GetByQueryFn by janisz · Pull Request #15010 · stackrox/stackrox

janisz · 2025-04-16T11:31:27Z

Description

The GetByQuery function retrieves a slice of objects from the database, which we often process item-by-item afterward. To improve performance, this update introduces support for a callback that is invoked during row iteration. This approach is similar to WalkByQuery, but it operates without using a database cursor.

Refs:

CHANGELOG update is not needed
Documentation is not needed

Testing

inspected CI results

Automated testing

modified existing tests
contributed no automated tests

How I validated my change

CI

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

sourcery-ai

Hey @janisz - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider adding a benchmark to measure the performance improvement of GetByQueryFn compared to GetByQuery.
It would be helpful to add a comment explaining why WalkByQuery isn't suitable for this use case.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

rhacs-bot · 2025-04-16T12:03:47Z

Images are ready for the commit at 89f5209.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-502-g89f5209512.

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

sourcery-ai

Hey @janisz - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider adding a comment to GetByQueryFn explaining when to prefer it over WalkByQuery.
It looks like you've added a GetByQueryFn test to every store - consider creating a shared test function.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟡 Testing: 1 issue found
🟡 Complexity: 1 issue found
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

pkg/search/postgres/store_test.go

pkg/search/postgres/store_cache.go

pkg/search/postgres/store.go

pkg/search/postgres/store_cache.go

codecov · 2025-04-17T11:04:10Z

Codecov Report

Attention: Patch coverage is 74.41860% with 11 lines in your changes missing coverage. Please review.

Project coverage is 49.05%. Comparing base (87c2ce5) to head (89f5209).
Report is 102 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/search/postgres/store_cache.go	72.50%	7 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #15010      +/-   ##
==========================================
+ Coverage   48.95%   49.05%   +0.10%     
==========================================
  Files        2549     2550       +1     
  Lines      187158   187373     +215     
==========================================
+ Hits        91615    91920     +305     
+ Misses      88302    88195     -107     
- Partials     7241     7258      +17

Flag	Coverage Δ
go-unit-tests	`49.05% <74.41%> (+0.10%)`	⬆️

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.

Co-authored-by: David Shrewsberry <99685630+dashrews78@users.noreply.github.com>

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

dashrews78

LGTM. Probably wise to get one of your teammates that were more involved in the cache store to approve before merging.

github-actions · 2025-04-22T12:16:06Z

Performance test results

Summary

  █ main dashboard

  █ list alerts grpc

    ✓ status is OK

  checks.........................: 100.00% ✓ 50       ✗ 0  
  data_received..................: 4.0 MB  64 kB/s
  data_sent......................: 240 kB  3.9 kB/s
  group_duration.................: avg=567.46ms min=85.41ms med=539.9ms max=1.22s    p(90)=1.07s    p(95)=1.14s   
  grpc_req_duration..............: avg=86.26ms  min=83.74ms med=86.34ms max=90.55ms  p(90)=88.26ms  p(95)=89.34ms 
  http_req_blocked...............: avg=170.54µs min=311ns   med=461ns   max=76.51ms  p(90)=691ns    p(95)=771ns   
  http_req_connecting............: avg=84.25µs  min=0s      med=0s      max=37.91ms  p(90)=0s       p(95)=0s      
  http_req_duration..............: avg=116ms    min=37.82ms med=82.03ms max=635.9ms  p(90)=456.07ms p(95)=486.56ms
    { expected_response:true }...: avg=138.23ms min=42.96ms med=83.86ms max=635.9ms  p(90)=467.53ms p(95)=495.06ms
  ✓ { lib:true }.................: avg=0s       min=0s      med=0s      max=0s       p(90)=0s       p(95)=0s      
  http_req_failed................: 22.22%  ✓ 100      ✗ 350
  http_req_receiving.............: avg=8.44ms   min=31.86µs med=72.46µs max=87.48ms  p(90)=74.72ms  p(95)=74.95ms 
  http_req_sending...............: avg=100.35µs min=44.13µs med=103.8µs max=384.38µs p(90)=126.96µs p(95)=135.83µs
  http_req_tls_handshaking.......: avg=85.34µs  min=0s      med=0s      max=38.4ms   p(90)=0s       p(95)=0s      
  http_req_waiting...............: avg=107.45ms min=37.69ms med=48.01ms max=635.74ms p(90)=455.88ms p(95)=486.33ms
  http_reqs......................: 450     7.220272/s
  iteration_duration.............: avg=1.24s    min=1.18s   med=1.22s   max=1.42s    p(90)=1.34s    p(95)=1.36s   
  iterations.....................: 50      0.802252/s
  vus............................: 1       min=1      max=1
  vus_max........................: 1       min=1      max=1

Sources

Summary: HTML Report | JSON
Raw Data: CSV | JSON

janisz · 2025-04-29T11:13:02Z

@rhybrillou @mtodor PTAL

rhybrillou

Please check my comments, I'll do another pass once these are addressed.

pkg/search/postgres/store_cache.go

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz · 2025-04-29T14:34:45Z

@rhybrillou fixed. We need some better way to get prealloc for pagination, so far I used batchAfter

janisz · 2025-04-30T11:18:24Z

/retest

pkg/search/postgres/store_cache.go

janisz added 2 commits April 16, 2025 13:09

ROX-28939: create GetByQueryFn

ef72d1b

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

deprecate

67ca4ad

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz requested review from a team as code owners April 16, 2025 11:31

janisz marked this pull request as draft April 16, 2025 11:31

openshift-ci bot added the do-not-merge/work-in-progress label Apr 16, 2025

janisz removed the request for review from a team April 16, 2025 11:31

github-actions bot added area/central area/postgres labels Apr 16, 2025

sourcery-ai bot reviewed Apr 16, 2025

View reviewed changes

generate

643ebd4

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz marked this pull request as ready for review April 17, 2025 09:58

openshift-ci bot removed the do-not-merge/work-in-progress label Apr 17, 2025

janisz requested review from mtodor and rhybrillou April 17, 2025 09:59

sourcery-ai bot reviewed Apr 17, 2025

View reviewed changes

pkg/search/postgres/store_test.go Show resolved Hide resolved

pkg/search/postgres/store_cache.go Show resolved Hide resolved

dashrews78 reviewed Apr 17, 2025

View reviewed changes

pkg/search/postgres/store.go Outdated Show resolved Hide resolved

dashrews78 reviewed Apr 17, 2025

View reviewed changes

pkg/search/postgres/store_cache.go Outdated Show resolved Hide resolved

janisz and others added 3 commits April 17, 2025 13:50

Update pkg/search/postgres/store.go

da53522

Co-authored-by: David Shrewsberry <99685630+dashrews78@users.noreply.github.com>

reuse code

5fbff12

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

add err test

14e1124

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz requested a review from dashrews78 April 17, 2025 12:14

dashrews78 approved these changes Apr 17, 2025

View reviewed changes

janisz added the ci-performance-tests Runs the K6 based performance tests label Apr 22, 2025

janisz mentioned this pull request Apr 25, 2025

ROX-28941: Use GetByQueryFn in alerts #15085

Merged

rhybrillou reviewed Apr 29, 2025

View reviewed changes

pkg/search/postgres/store_cache.go Show resolved Hide resolved

pkg/search/postgres/store_cache.go Outdated Show resolved Hide resolved

pkg/search/postgres/store_cache.go Outdated Show resolved Hide resolved

rhybrillou reviewed Apr 29, 2025

View reviewed changes

pkg/search/postgres/store_cache.go Show resolved Hide resolved

fix

89f5209

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz requested a review from rhybrillou April 29, 2025 14:57

rhybrillou approved these changes Apr 30, 2025

View reviewed changes

pkg/search/postgres/store_cache.go Show resolved Hide resolved

janisz merged commit 76d1864 into master Apr 30, 2025
93 checks passed

janisz deleted the ROX-28939_create_getbyqueryfn branch April 30, 2025 17:03

janisz mentioned this pull request Sep 23, 2025

fix(risk): improve reprocess deployment risk #16987

Closed

This was referenced Oct 3, 2025

ROX-31176: improve reprocess deployment risk #17126

Merged

ROX-31175: Prefer RunSelectRequestForSchemaFn #17141

Open

Conversation

janisz commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Automated testing

How I validated my change

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

rhacs-bot commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dashrews78 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance test results

Summary

Sources

Uh oh!

janisz commented Apr 29, 2025

Uh oh!

rhybrillou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janisz commented Apr 29, 2025

Uh oh!

janisz commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

janisz commented Apr 16, 2025 •

edited

Loading

rhacs-bot commented Apr 16, 2025 •

edited

Loading

codecov bot commented Apr 17, 2025 •

edited

Loading

github-actions bot commented Apr 22, 2025 •

edited

Loading