Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • added redis option for rate limiter, 10x speed improvement in rate limit checks & reduction of DB load
  • falls back to DB for self-hosters who don't use redis, no disruption

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…nt in rate limit checks & reduction of DB load
@vercel
Copy link

vercel bot commented Dec 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 9, 2025 4:31am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 9, 2025

Greptile Overview

Greptile Summary

Implemented Redis-based rate limiting with atomic Lua script operations, achieving 10x performance improvement over the DB-only approach. The implementation includes graceful fallback to database when Redis is unavailable, ensuring no disruption for self-hosters.

Key improvements:

  • Redis rate limiting uses atomic INCR + EXPIRE in a single Lua script to prevent race conditions
  • Database fallback maintains existing functionality for environments without Redis
  • Rate limiter module relocated from services/queue to lib/core/rate-limiter for better organization
  • Comprehensive test coverage for both Redis and DB paths, including failure scenarios
  • Manual executions continue to bypass rate limiting as before

Architecture:

  • Fixed-window rate limiting algorithm with per-minute windows
  • Separate counters for sync/async API executions and API endpoint requests
  • Organization-level rate limit pooling for team/enterprise subscriptions
  • Automatic fallback on Redis errors with warning logs

Confidence Score: 4/5

  • Safe to merge with minor considerations about Redis connection handling in production
  • The implementation properly addresses previous race condition issues with atomic Lua operations, includes comprehensive fallback logic, and has good test coverage. Score is 4 (not 5) due to lack of Redis integration tests and potential edge cases around Redis connection lifecycle in serverless environments
  • apps/sim/lib/core/rate-limiter/rate-limiter.ts - verify Redis connection behavior under load

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/core/rate-limiter/rate-limiter.ts 4/5 New Redis-based rate limiter with atomic Lua script operations and DB fallback; previous race conditions addressed
apps/sim/lib/core/rate-limiter/rate-limiter.test.ts 5/5 Comprehensive test coverage for Redis and DB fallback paths, Redis failure scenarios, and different trigger types
apps/sim/app/api/v1/middleware.ts 5/5 Import path updated to new rate limiter location
apps/sim/lib/execution/preprocessing.ts 5/5 Import path updated to new rate limiter location

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as API Route/Middleware
    participant RL as RateLimiter
    participant Redis as Redis Client
    participant DB as Database
    
    Client->>API: API Request
    API->>RL: checkRateLimitWithSubscription()
    
    alt Manual Trigger
        RL->>API: allowed=true (bypass)
    else API/Webhook/Schedule Trigger
        RL->>Redis: getRedisClient()
        
        alt Redis Available
            Redis-->>RL: Redis instance
            RL->>Redis: eval(Lua: INCR + EXPIRE)
            Redis-->>RL: count
            
            alt count <= limit
                RL->>API: allowed=true, remaining
            else count > limit
                RL->>API: allowed=false, remaining=0
            end
            
        else Redis Unavailable/Error
            Redis-->>RL: null or error
            RL->>DB: SELECT rate limit record
            
            alt Record Expired/Missing
                RL->>DB: INSERT with onConflictDoUpdate
                DB-->>RL: new count
            else Record Valid
                RL->>DB: UPDATE (atomic increment)
                DB-->>RL: updated count
            end
            
            alt count <= limit
                RL->>API: allowed=true, remaining
            else count > limit
                RL->>DB: UPDATE isRateLimited=true
                RL->>API: allowed=false, remaining=0
            end
        end
    end
    
    API->>Client: Response
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

12 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 9f0584a into staging Dec 9, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/redis-rl branch December 9, 2025 04:39
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