Skip to content

ci: Add simulated DB delay to performance benchmarks #9924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+37 −3
Closed

ci: Add simulated DB delay to performance benchmarks #9924

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions .github/workflows/ci-performance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,31 @@ jobs:
- name: Build Parse Server (base)
run: npm run build

- name: Setup Toxiproxy for latency simulation (baseline)
run: |
# Download and install toxiproxy
wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v2.9.0/toxiproxy_2.9.0_amd64.deb
Copy link

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hard-coded Toxiproxy version requires a maintenance plan.

The Toxiproxy version is hard-coded to v2.9.0 with no documented strategy for updates or deprecation handling. As CI infrastructure evolves, this could become a source of maintenance debt or security issues.

Consider using a version variable pinned at the top of the workflow for easier updates, or document the maintenance cadence.

Apply this diff to add versioning at the workflow level:

 env:
   NODE_VERSION: 24.11.0
   MONGODB_VERSION: 8.0.4
+  TOXIPROXY_VERSION: 2.9.0

Then update the setup step to reference it:

-         wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v2.9.0/toxiproxy_2.9.0_amd64.deb
-         sudo dpkg -i toxiproxy.deb
+         wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v${TOXIPROXY_VERSION}/toxiproxy_${TOXIPROXY_VERSION}_amd64.deb
+         sudo dpkg -i toxiproxy.deb

Committable suggestion skipped: line range outside the PR's diff.

sudo dpkg -i toxiproxy.deb
Comment on lines +71 to +72
Copy link

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add checksum validation for downloaded Toxiproxy binary.

Downloading executables without verifying integrity is a supply-chain risk. If the GitHub release is compromised or a MITM attack occurs, malicious code could run in the CI environment.

Consider adding checksum validation:

         wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v${TOXIPROXY_VERSION}/toxiproxy_${TOXIPROXY_VERSION}_amd64.deb
+        wget -O toxiproxy.deb.sha256 https://github.com/Shopify/toxiproxy/releases/download/v${TOXIPROXY_VERSION}/toxiproxy_${TOXIPROXY_VERSION}_amd64.deb.sha256
+        sha256sum -c toxiproxy.deb.sha256 || (echo "Checksum validation failed"; exit 1)
         sudo dpkg -i toxiproxy.deb

Alternatively, use a package manager or container image that handles verification automatically.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
.github/workflows/ci-performance.yml around lines 71-72: the workflow downloads
and installs the Toxiproxy .deb without verifying integrity; update the job to
validate the download by obtaining the expected checksum (preferably a vetted
SHA256 from a trusted source or a checksum file committed to the repo), compute
the SHA256 of the downloaded toxiproxy.deb in CI and compare it against the
expected value, and fail the job if they differ; alternatively replace the
direct download with a verified source such as the OS package manager or a
signed container image that provides built-in verification.


# Start toxiproxy server in background
toxiproxy-server &
sleep 2

# Create a proxy for MongoDB (27017 -> 27018)
toxiproxy-cli create mongodb -l localhost:27018 -u localhost:27017

# Add 10ms latency to simulate network conditions
toxiproxy-cli toxic add mongodb -t latency -a latency=10

echo "Toxiproxy setup complete - MongoDB accessible on port 27018 with 10ms latency"

Comment on lines +68 to +85
Copy link

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add error handling and health checks to Toxiproxy setup.

The Toxiproxy setup lacks error handling and health checks. If the download fails, the dpkg command fails, or the server doesn't start properly, the workflow continues silently and benchmarks run against an unavailable proxy, producing invalid results.

Additionally, the 2-second sleep after starting the server is arbitrary and may not be sufficient for the proxy to be ready in all CI conditions.

Apply this diff to add error handling and health checks:

      - name: Setup Toxiproxy for latency simulation (baseline)
        run: |
+         set -e
+         trap 'echo "Toxiproxy setup failed"; exit 1' ERR
+
          # Download and install toxiproxy
          wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v2.9.0/toxiproxy_2.9.0_amd64.deb
          sudo dpkg -i toxiproxy.deb
 
          # Start toxiproxy server in background
          toxiproxy-server &
-         sleep 2
+         sleep 3
+         
+         # Wait for toxiproxy to be ready (health check)
+         for i in {1..10}; do
+           if nc -z localhost 8474 2>/dev/null; then
+             echo "Toxiproxy server is ready"
+             break
+           fi
+           echo "Waiting for Toxiproxy server... ($i/10)"
+           sleep 1
+         done
 
          # Create a proxy for MongoDB (27017 -> 27018)
          toxiproxy-cli create mongodb -l localhost:27018 -u localhost:27017
+         if [ $? -ne 0 ]; then
+           echo "Failed to create MongoDB proxy"
+           exit 1
+         fi
 
          # Add 10ms latency to simulate network conditions
          toxiproxy-cli toxic add mongodb -t latency -a latency=10
+         if [ $? -ne 0 ]; then
+           echo "Failed to add latency toxic"
+           exit 1
+         fi
 
+         # Verify the proxy is working
+         if nc -z localhost 27018 2>/dev/null; then
+           echo "Toxiproxy setup complete - MongoDB accessible on port 27018 with 10ms latency"
+         else
+           echo "Toxiproxy proxy is not accessible on port 27018"
+           exit 1
+         fi
-         echo "Toxiproxy setup complete - MongoDB accessible on port 27018 with 10ms latency"

Note: This assumes netcat (nc) is available; you may need to install it or use an alternative connectivity test.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Setup Toxiproxy for latency simulation (baseline)
run: |
# Download and install toxiproxy
wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v2.9.0/toxiproxy_2.9.0_amd64.deb
sudo dpkg -i toxiproxy.deb
# Start toxiproxy server in background
toxiproxy-server &
sleep 2
# Create a proxy for MongoDB (27017 -> 27018)
toxiproxy-cli create mongodb -l localhost:27018 -u localhost:27017
# Add 10ms latency to simulate network conditions
toxiproxy-cli toxic add mongodb -t latency -a latency=10
echo "Toxiproxy setup complete - MongoDB accessible on port 27018 with 10ms latency"
- name: Setup Toxiproxy for latency simulation (baseline)
run: |
set -e
trap 'echo "Toxiproxy setup failed"; exit 1' ERR
# Download and install toxiproxy
wget -O toxiproxy.deb https://github.com/Shopify/toxiproxy/releases/download/v2.9.0/toxiproxy_2.9.0_amd64.deb
sudo dpkg -i toxiproxy.deb
# Start toxiproxy server in background
toxiproxy-server &
sleep 3
# Wait for toxiproxy to be ready (health check)
for i in {1..10}; do
if nc -z localhost 8474 2>/dev/null; then
echo "Toxiproxy server is ready"
break
fi
echo "Waiting for Toxiproxy server... ($i/10)"
sleep 1
done
# Create a proxy for MongoDB (27017 -> 27018)
toxiproxy-cli create mongodb -l localhost:27018 -u localhost:27017
if [ $? -ne 0 ]; then
echo "Failed to create MongoDB proxy"
exit 1
fi
# Add 10ms latency to simulate network conditions
toxiproxy-cli toxic add mongodb -t latency -a latency=10
if [ $? -ne 0 ]; then
echo "Failed to add latency toxic"
exit 1
fi
# Verify the proxy is working
if nc -z localhost 27018 2>/dev/null; then
echo "Toxiproxy setup complete - MongoDB accessible on port 27018 with 10ms latency"
else
echo "Toxiproxy proxy is not accessible on port 27018"
exit 1
fi

- name: Run baseline benchmarks
id: baseline
env:
NODE_ENV: production
MONGODB_URI: mongodb://localhost:27018/parse_benchmark_test
run: |
echo "Running baseline benchmarks with CPU affinity (using PR's benchmark script)..."
echo "Running baseline benchmarks with CPU affinity and 10ms DB latency (using PR's benchmark script)..."
if [ ! -f "benchmark/performance.js" ]; then
echo "⚠️ Benchmark script not found - this is expected for new features"
echo "Skipping baseline benchmark"
Expand Down Expand Up @@ -130,12 +149,27 @@ jobs:
- name: Build Parse Server (PR)
run: npm run build

- name: Restart Toxiproxy for PR benchmarks
run: |
# Restart toxiproxy server (toxiproxy-cli is already installed from baseline)
pkill toxiproxy-server || true
sleep 1
toxiproxy-server &
sleep 2

# Recreate proxy with same configuration
toxiproxy-cli create mongodb -l localhost:27018 -u localhost:27017
toxiproxy-cli toxic add mongodb -t latency -a latency=10

echo "Toxiproxy restarted with 10ms latency"

Comment on lines +152 to +165
Copy link

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add health checks and error handling to Toxiproxy restart step.

The restart step uses pkill ... || true which silently suppresses errors, potentially leaving stale processes. There's no verification that the old process actually stopped or that the new one starts correctly. This can cause state corruption or port conflicts.

Apply this diff to add error handling and validation:

      - name: Restart Toxiproxy for PR benchmarks
        run: |
+         set -e
+         trap 'echo "Toxiproxy restart failed"; exit 1' ERR
+
          # Restart toxiproxy server (toxiproxy-cli is already installed from baseline)
-         pkill toxiproxy-server || true
-         sleep 1
-         toxiproxy-server &
-         sleep 2
+         if pgrep -f toxiproxy-server > /dev/null; then
+           echo "Stopping existing Toxiproxy instance..."
+           pkill -f toxiproxy-server
+           sleep 2
+           # Verify the old process is gone
+           if pgrep -f toxiproxy-server > /dev/null; then
+             echo "Warning: Toxiproxy process still running, using pkill -9"
+             pkill -9 -f toxiproxy-server
+             sleep 1
+           fi
+         fi
+
+         echo "Starting Toxiproxy server..."
+         toxiproxy-server &
+         TOXIPROXY_PID=$!
+         sleep 3
+         
+         # Wait for toxiproxy to be ready
+         for i in {1..10}; do
+           if nc -z localhost 8474 2>/dev/null; then
+             echo "Toxiproxy server is ready (PID: $TOXIPROXY_PID)"
+             break
+           fi
+           echo "Waiting for Toxiproxy server... ($i/10)"
+           sleep 1
+         done
 
          # Recreate proxy with same configuration
          toxiproxy-cli create mongodb -l localhost:27018 -u localhost:27017
          toxiproxy-cli toxic add mongodb -t latency -a latency=10
+         if [ $? -ne 0 ]; then
+           echo "Failed to recreate MongoDB proxy"
+           exit 1
+         fi
 
-         echo "Toxiproxy restarted with 10ms latency"
+         # Verify the proxy is working
+         if nc -z localhost 27018 2>/dev/null; then
+           echo "Toxiproxy restarted with 10ms latency - MongoDB accessible on port 27018"
+         else
+           echo "Toxiproxy proxy is not accessible on port 27018 after restart"
+           exit 1
+         fi
🤖 Prompt for AI Agents 
.github/workflows/ci-performance.yml around lines 152 to 165: the Toxiproxy
restart step lacks verification and silences failures; update the step to
explicitly stop any existing toxiproxy-server and fail if it cannot be
terminated within a short timeout, verify the proxy port is free before
starting, start toxiproxy-server and poll until it responds on its control port
(failing the job on timeout), recreate the proxy and toxic but check the return
codes or HTTP responses for success and retry a couple times before failing, and
emit clear error messages on each failure so the CI job fails fast on
unrecoverable errors.

- name: Run PR benchmarks
id: pr-bench
env:
NODE_ENV: production
MONGODB_URI: mongodb://localhost:27018/parse_benchmark_test
run: |
echo "Running PR benchmarks with CPU affinity..."
echo "Running PR benchmarks with CPU affinity and 10ms DB latency..."
taskset -c 0 npm run benchmark > pr-output.txt 2>&1 || npm run benchmark > pr-output.txt 2>&1 || true
echo "Benchmark command completed with exit code: $?"
echo "Output file size: $(wc -c < pr-output.txt) bytes"
Expand Down Expand Up @@ -306,7 +340,7 @@ jobs:
echo "" >> comment.md
echo "*Benchmarks ran with ${BENCHMARK_ITERATIONS:-1000} iterations per test on Node.js ${{ env.NODE_VERSION }} (production mode, CPU pinned)*" >> comment.md
echo "" >> comment.md
echo "> **Note:** Using 1k iterations with CPU affinity for measurement stability. Thresholds: ⚠️ >50%, ❌ >100%." >> comment.md
echo "> **Note:** Both baseline and PR benchmarks run with 10ms simulated database latency to better measure optimization impact. Using 1k iterations with CPU affinity for measurement stability. Thresholds: ⚠️ >50%, ❌ >100%." >> comment.md

- name: Comment PR with results
if: github.event_name == 'pull_request'
Expand Down
Loading