Skip to content

Conversation

@manish-singh-bisht
Copy link
Contributor

@manish-singh-bisht manish-singh-bisht commented Dec 16, 2025

Description

Fixes #654

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  1. tested locally for the functionality, added lot of rows in the db to get the rps as in images below.

  2. also have tested this in the github actions in my own fork, https://github.com/manish-singh-bisht/olake/actions/runs/20350650406/job/58474101019 , only for postgres

failed rps rps data rps success

Screenshots or Recordings

Documentation

  • Documentation Link: [link to README, olake.io/docs, or olake-docs]
  • N/A (bug fix, refactor, or test changes only)

Related PR's (If Any):

@manish-singh-bisht manish-singh-bisht changed the title WIP feat: dynamic rps benchmarking feat: dynamic rps benchmarking Dec 18, 2025
@manish-singh-bisht manish-singh-bisht marked this pull request as ready for review December 18, 2025 20:47
@manish-singh-bisht
Copy link
Contributor Author

@vishalm0509 ready for review.

@nayanj98 nayanj98 requested a review from vishalm0509 December 22, 2025 06:18
@vishalm0509
Copy link
Collaborator

Self-note / future idea to explore:

Consider changing the artifact download logic so that we don’t only read from the “last successful” workflow run. Instead, always download the most recent run’s artifact for each driver (regardless of success/failure), and then only update/write the benchmarks.json when that driver’s performance test actually passes.
Implications:
• If MySQL fails but Postgres succeeds in a mixed run, Postgres would update its baseline while MySQL would keep its previous one.
• This gives us per-driver history continuity even when the workflow overall is partially failing.
• Worth thinking through edge cases and implementation complexity before proceeding.

Just noting it here, let's proceed after internal discussions

@vishalm0509
Copy link
Collaborator

vishalm0509 commented Jan 10, 2026

@manish-singh-bisht can we move all the methods from benchmark.go to test_utils.go itself ?

@manish-singh-bisht
Copy link
Contributor Author

@manish-singh-bisht can we move all the methods from benchmark.go to test_utils.go itself ?

shifted

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.

2 participants