Skip to content

Conversation

@nirmitparikh8
Copy link
Contributor

@nirmitparikh8 nirmitparikh8 commented Nov 3, 2025

fixes #6619

Why This Is Important

This PR establishes automated validation for the adaptive PID controller circuit breaker implementation, ensuring performance regression detection and providing continuous benchmarking against classic circuit breakers across multiple failure scenarios.

What We're Accomplishing

  • Continuous Integration: Testing of PID controller and classic circuit breaker performance on every PR
  • Performance Validation: Detection of regressions in circuit breaker behavior
  • Visual Documentation: Ensuring performance graphs are updated with every commit

Key Changes Made

1. CI Workflow Setup

  • Created .github/workflows/automated-experiment-result-checkeryml
  • Check if latest commit updated all the graphs that indicate true performance of teh circuit breaker. If any images are outdated CI fails and tells the user what command to run.

2. Experiment Infrastructure Refactor

  • Reorganized experiment structure: Moved test files to experiments/experiments/ directory and results into experiments/results/ directory
  • Parallel test execution: Created run_all_experiments.rb with threaded execution (14 concurrent tests)
  • Automated graph generation (requests, duration, throughput)

3. Test Coverage

14 test scenarios comparing adaptive vs classic circuit breakers:

  • Sustained load (20% error rate)
  • Error spikes (20% and 100% rates)
  • Gradual error increases
  • Oscillating error patterns
  • Multiple service degradation scenarios

NOTE: We skip the lower_bound_adaptive test for the PID controller because it takes an hour. We will create a ticket to make it shorter or more optimized and then include it.

Future Steps

  • Block merging to Semian main branch if all CI checks don't pass.
  • Set something up where we can comment on the PR to generate updated graphs and commit back to the PR

This new check will automatically help catch any regressions introduced by code changes.

@nirmitparikh8 nirmitparikh8 changed the base branch from main to pid-take-2 November 3, 2025 19:55
@nirmitparikh8 nirmitparikh8 force-pushed the add-automated-workflow-for-pid branch 3 times, most recently from 92371ea to b88adda Compare November 4, 2025 14:20
@nirmitparikh8 nirmitparikh8 force-pushed the add-automated-workflow-for-pid branch 3 times, most recently from cf1475a to 1f04a67 Compare November 4, 2025 21:39
@nirmitparikh8 nirmitparikh8 force-pushed the add-automated-workflow-for-pid branch from bbaae80 to cca4c55 Compare November 5, 2025 16:41
@nirmitparikh8 nirmitparikh8 self-assigned this Nov 5, 2025
Copy link
Contributor

@AbdulRahmanAlHamali AbdulRahmanAlHamali left a comment

Choose a reason for hiding this comment

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

I added a bunch of comments. But looks good overall!

A couple of thoughts:

  1. It seems CI did not run on your latest commit. I wonder if the last commit being made by the github action bot s blocking CI. Could you confirm?
  2. Also, we probably wanna do a fast followup after this that adds a CI that basically confirms that all the experiments have been rerun (basically by confirming that every PR is updating all existing results).

@nirmitparikh8 nirmitparikh8 force-pushed the add-automated-workflow-for-pid branch 5 times, most recently from dfdc165 to d65fa1d Compare November 7, 2025 16:19
Reset

Update experiment graphs

Delete existing image

Done

Update experiment graphs

Update experiment graphs

Refactor experiments folder and CI

Update scope of commit

Update experiment graphs

Update experiment graphs

Update Gemfile

Update experiment graphs

Move windup file into test folder

Update experiment graphs

CI should only commit main graphs

Update experiment graphs

New bot

Fail fast

Test

Delete comment

Update experiment graphs

test

Revert

test

Add the commit

Test

Test
Done

Final
@nirmitparikh8 nirmitparikh8 force-pushed the add-automated-workflow-for-pid branch from e42334d to ac1de1e Compare November 7, 2025 17:19
@nirmitparikh8 nirmitparikh8 merged commit cb2690f into pid-take-2 Nov 7, 2025
32 checks passed
@nirmitparikh8 nirmitparikh8 deleted the add-automated-workflow-for-pid branch November 7, 2025 18:29
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