Skip to content
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.

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

feat: Add solver rate limiter #419

Merged
merged 4 commits into from
Nov 1, 2024
Merged

Conversation

bgins
Copy link
Contributor

@bgins bgins commented Oct 30, 2024

Summary

This pull request makes the following changes:

  • Add httprate limiter to solver server
  • Add server rate limiter options
  • Add extra args to stack solver command
  • Add rate limiter integration test

We would like to limit the number of requests from an IP address by route.

Task/Issue reference

Closes: #417

Test plan

Start the chain and solver nodes. The other parts of stack will not be needed for testing.

Copy this script into a push-limits.go file:

package main

import (
	"fmt"
	"io"
	"net/http"
	"os"
	"sync"
	"time"
)

func main() {
	configs := map[string]struct {
		path         string
		initialDelay int // in milliseconds
	}{
		"resource_offers": {path: "/api/v1/resource_offers", initialDelay: 0},
		"job_offers":      {path: "/api/v1/job_offers", initialDelay: 1000},
		"deals":           {path: "/api/v1/deals", initialDelay: 2000},
	}

	var wg sync.WaitGroup
	var mu sync.Mutex

	// Send off callers to run concurrently
	for _, config := range configs {
		wg.Add(1)

		go func() {
			defer wg.Done()
			makeCalls(config.path, config.initialDelay, &mu)
		}()
	}

	wg.Wait()
}

func makeCalls(path string, initialDelay int, mu *sync.Mutex) {
	// Wait a bit to stagger the callers
	time.Sleep(time.Duration(initialDelay) * time.Millisecond)

        // Make 10 requests
	for i := range 10 {
		requestURL := fmt.Sprintf("http://localhost:%d%s", 8080, path)
		res, err := http.Get(requestURL)

		if err != nil {
			fmt.Printf("get request failed on %s: %s\n", path, err)
			os.Exit(1)
		}

		// Ensure full result printed in order
		mu.Lock()
		printResult(path, res, i)
		mu.Unlock()

		// Wait before making next call
		time.Sleep(300 * time.Millisecond)
	}

}

func printResult(path string, res *http.Response, count int) {
	fmt.Printf("path: %v\n", path)
	fmt.Printf("status code: %d\n", res.StatusCode)
	fmt.Printf("count: %d\n", count+1)

	if res.StatusCode == 429 {
		resBody, err := io.ReadAll(res.Body)
		if err != nil {
			fmt.Printf("could not read response body: %s\n", err)
			os.Exit(1)
		}

		fmt.Printf("error body: %s\n", resBody)
	} else {
		fmt.Println()
	}
}

This script runs 10 requests across our get resource offer, job offer, and deals endpoints. The makeCalls for each endpoint are staggered so they can be viewed independently in the output.

Run the script with go run push-limits.go.

The expected output is successful calls to get resource offers at first:

path: /api/v1/resource_offers
status code: 200
count: 1

We default to five requests allowed over 10 seconds. Once an endpoint has reached it's limit, it should report 429s:

path: /api/v1/resource_offers
status code: 429
count: 6
error body: Too Many Requests

The outputs for the job offer and deals endpoints will be interleaved, but should demonstrate that the rate limit is per endpoint and not global.

Test the rate limiting configuration by starting the solver with CLI options or environment variables:

./stack solver --server-rate-request-limit 1 --server-rate-window-length 5
SERVER_RATE_REQUEST_LIMIT=1 SERVER_RATE_WINDOW_LENGTH=5 ./stack solver

Run the push-limits.go script again and the limits should be enforced much sooner.

Details

We considered rate limiting by wallet address, but have decided to use IP address for a first pass. Some endpoints do not require our X-Lilypad-User header, so wallet address is not sufficient. We may want to revisit this idea in the future.

In addition to httprate, we also considered tollbooth. tollbooth is more widely used, but httprate has a better algorithm based on work at Cloudflare. Also, httprate supports Redis for tracking counts over multiple server instances.

The SERVER_RATE_REQUEST_LIMIT and SERVER_RATE_WINDOW_LENGTH environment variables have been added to all Doppler solver envinronments with the default values. We can update these values and restart the solver to tune them.

@cla-bot cla-bot bot added the cla-signed label Oct 30, 2024
@@ -217,7 +217,7 @@ function solver() {
load-local-env
export WEB3_PRIVATE_KEY=${SOLVER_PRIVATE_KEY}
export LOG_LEVEL=debug
go run . solver --network dev
go run . solver --network dev "$@"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This adds on extra args from the command line. We had it on other services, but it was missing on the solver.

@bgins bgins marked this pull request as ready for review October 30, 2024 20:55
@bgins bgins requested a review from a team as a code owner October 30, 2024 20:55
@bgins bgins changed the title Bgins/feat add solver rate limiter feat: Add solver rate limiter Oct 30, 2024
Copy link
Collaborator

@walkah walkah left a comment

Choose a reason for hiding this comment

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

This looks great! nice and clean. Tested locally and it's working.

Would it be possible to make push-limits a test so we can run them on PRs to make sure we don't regress? (if it's a big lift, we can add a task on the backlog)

@bgins bgins force-pushed the bgins/feat-add-solver-rate-limiter branch from d00a25e to 0e8a7f3 Compare October 31, 2024 21:10
@bgins
Copy link
Contributor Author

bgins commented Oct 31, 2024

Would it be possible to make push-limits a test so we can run them on PRs to make sure we don't regress? (if it's a big lift, we can add a task on the backlog)

Yeah, great idea! Adapted it into a test here: 0e8a7f3

@bgins bgins force-pushed the bgins/feat-add-solver-rate-limiter branch from 0e8a7f3 to 35e9a8d Compare October 31, 2024 21:32
@bgins bgins force-pushed the bgins/feat-add-solver-rate-limiter branch from 45e2056 to 1556f3f Compare October 31, 2024 22:49
@bgins bgins merged commit d30b6d8 into main Nov 1, 2024
5 checks passed
@bgins bgins deleted the bgins/feat-add-solver-rate-limiter branch November 1, 2024 02:21
walkah pushed a commit that referenced this pull request Dec 10, 2024
* chore: Add extra args to solver command

* chore: Add server rate limiter options

* feat: Add httprate limiter to solver server

* test: Add rate limiter integration test
@bgins bgins self-assigned this Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add solver HTTP rate limiting
3 participants