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

chore: Migrate redigo to go-redis, upgrade throttled #2737

Merged
merged 5 commits into from
Jan 28, 2025
Merged

Conversation

mschfh
Copy link
Collaborator

@mschfh mschfh commented Jan 21, 2025

Summary

This PR replaces our outdated redigo dependency with go-redis.

Type of Change

  • Product feature
  • Bug fix
  • Performance improvement
  • Refactor
  • Other

Tested Environments

  • Development
  • Staging
  • Production

Before Requesting Review

  • Does your code build cleanly without any errors or warnings?
  • Have you used auto closing keywords?
  • Have you added tests for new functionality?
  • Have validated query efficiency for new database queries?
  • Have documented new functionality in README or in comments?
  • Have you squashed all intermediate commits?
  • Is there a clear title that explains what the PR does?
  • Have you used intuitive function, variable and other naming?
  • Have you requested security and/or privacy review if needed
  • Have you performed a self review of this PR?

@mschfh mschfh self-assigned this Jan 21, 2025
@mschfh mschfh force-pushed the mschfh-goredis branch 2 times, most recently from b8273b3 to 9db4ff3 Compare January 22, 2025 10:05
@mschfh mschfh marked this pull request as ready for review January 22, 2025 10:07
@mschfh mschfh requested a review from clD11 January 22, 2025 10:08
Copy link
Contributor

@clD11 clD11 left a comment

Choose a reason for hiding this comment

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

LGTM approved with comments.

libs/clients/coingecko/client.go Outdated Show resolved Hide resolved
libs/clients/coingecko/client.go Show resolved Hide resolved
libs/clients/coingecko/client.go Outdated Show resolved Hide resolved
libs/clients/coingecko/client_test.go Outdated Show resolved Hide resolved
libs/clients/coingecko/client_test.go Outdated Show resolved Hide resolved
libs/middleware/rate_limiter.go Outdated Show resolved Hide resolved
libs/middleware/rate_limiter.go Outdated Show resolved Hide resolved
services/ratios/cache.go Show resolved Hide resolved
services/ratios/cache.go Outdated Show resolved Hide resolved
services/ratios/cache.go Outdated Show resolved Hide resolved
@mschfh mschfh requested a review from jen140 as a code owner January 28, 2025 21:46
@mschfh mschfh enabled auto-merge (squash) January 28, 2025 21:46
Copy link

[puLL-Merge] - brave-intl/bat-go@2737

Description

This PR upgrades Redis client library from redigo to go-redis/v9 across the codebase. The change also includes updating the Dockerfile to use build caching and fixing some formatting issues with container stage names. The motivation appears to be modernizing the Redis client usage and improving build performance.

Possible Issues

  1. The Redis cache writes no longer specify TTL/expiration times which were previously handled by Redis defaults. This could lead to memory leaks if keys are not properly managed.

  2. The removal of Redis connection pooling configuration (previously handled by redigo) means connection management is now entirely handled by go-redis. This may require tuning in production environments.

Security Hotspots

None significant - the changes mainly involve refactoring Redis client usage with equivalent functionality.

Changes

Changes

By filename:

Dockerfile:

  • Updated stage naming format (lowercase to uppercase)
  • Added build cache mount for Go modules
  • Minor formatting changes

libs/clients/coingecko/client.go:

  • Replaced redigo Redis client with go-redis/v9
  • Updated Redis operations to use new client API
  • Removed connection pool management code

libs/middleware/rate_limiter.go:

  • Updated rate limiter to use throttled/v2 package
  • Changed Redis store implementation to use go-redis/v9
  • Simplified rate limiter configuration

services/ratios/cache.go:

  • Migrated Redis operations from redigo to go-redis/v9
  • Updated sorted set operations syntax
  • Removed explicit connection management
sequenceDiagram
    participant App
    participant Redis
    participant Coingecko
    
    App->>Redis: Connect (using go-redis/v9)
    Note over Redis: New connection pool managed by go-redis
    
    App->>Redis: Check cache for market data
    
    alt Cache Miss
        App->>Coingecko: Fetch latest data
        Coingecko-->>App: Return market data
        App->>Redis: Store in cache
    else Cache Hit
        Redis-->>App: Return cached data
    end
Loading

@mschfh mschfh merged commit fbdf2fe into master Jan 28, 2025
13 checks passed
@mschfh mschfh deleted the mschfh-goredis branch January 28, 2025 21:51
@clD11 clD11 mentioned this pull request Jan 29, 2025
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.

5 participants