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

[Feature Request] Refactor Redis Connection #226

Closed
1 task done
H0llyW00dzZ opened this issue Apr 11, 2024 · 3 comments
Closed
1 task done

[Feature Request] Refactor Redis Connection #226

H0llyW00dzZ opened this issue Apr 11, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@H0llyW00dzZ
Copy link
Contributor

Tell us about your feature request

The current Redis environment and the way it connects keeps showing an EOF error during initialization. This issue needs to be addressed to ensure a stable and reliable connection to the Redis server.

Current Behavior
When running the application, the following error is encountered:

$ go run cmd/api/main.go
2024/04/11 19:25:42 [H0llyW00dzZ Firewall] [INFO] Initialization to database: *********
2024/04/11 19:25:45 [H0llyW00dzZ Firewall] [INFO] Successfully initialized the firewall table.
2024/04/11 19:25:48 [H0llyW00dzZ Rate Limiter] [FATAL] Failed to initialize redis service: failed to connect to redis: EOF
exit status 1

Code Snippet
The current implementation of the Redis connection is as follows:

var (
    address  = os.Getenv("DB_ADDRESS")
    port     = os.Getenv("DB_PORT")
    password = os.Getenv("DB_PASSWORD")
    database = os.Getenv("DB_DATABASE")
)

func New() Service {
    num, err := strconv.Atoi(database)
    if err != nil {
        log.Fatalf(fmt.Sprintf("database incorrect %v", err))
    }

    fullAddress := fmt.Sprintf("%s:%s", address, port)

    rdb := redis.NewClient(&redis.Options{
        Addr:     fullAddress,
        Password: password,
        DB:       num,
    })

    s := &service{db: rdb}

    return s
}

Proposed Solution
To resolve the issue, the Redis connection needs to be established with the appropriate TLS configuration. By adding the TLSConfig option with the minimum TLS version set to tls.VersionTLS12, the connection was successfully established.

Here's the corrected code:

var (
    address  = os.Getenv("DB_ADDRESS")
    port     = os.Getenv("DB_PORT")
    password = os.Getenv("DB_PASSWORD")
    database = os.Getenv("DB_DATABASE")
)

func New() (Service, error) {
    num, err := strconv.Atoi(database)
    if err != nil {
        return nil, fmt.Errorf("database incorrect %v", err)
    }

    fullAddress := fmt.Sprintf("%s:%s", address, port)

    rdb := redis.NewClient(&redis.Options{
        Addr:     fullAddress,
        Password: password,
        DB:       num,
        TLSConfig: &tls.Config{
            MinVersion: tls.VersionTLS12,
        },
    })

    // Test connection
    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
    defer cancel()

    pong, err := rdb.Ping(ctx).Result()
    if err != nil {
        return nil, fmt.Errorf("failed to connect to redis: %v", err)
    }

    log.Printf("Redis ping response: %s", pong)

    s := &service{db: rdb}

    return s, nil
}

Additionally, a connection test has been added to ensure that the Redis connection is established successfully before proceeding with the application.

Testing
The following code was used for testing the Redis connection during initialization:

// Test connection
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

pong, err := rdb.Ping(ctx).Result()
if err != nil {
    return nil, fmt.Errorf("failed to connect to redis: %v", err)
}

log.Printf("Redis ping response: %s", pong)

With the updated code and the added TLS configuration, the Redis connection is now established successfully, and the ping response is logged as expected.

Log:

$ go run cmd/api/main.go
2024/04/11 19:37:39 [H0llyW00dzZ Firewall] [INFO] Initialization to database: *********
2024/04/11 19:37:41 [H0llyW00dzZ Firewall] [INFO] Successfully initialized the firewall table.
2024/04/11 19:37:44 [H0llyW00dzZ Rate Limiter] Redis ping response: PONG
2024/04/11 19:37:45 [H0llyW00dzZ Project] [INFO] Starting server on :8080

 ┌───────────────────────────────────────────────────┐
 │                    H0llyW00dzZ                    │
 │                   Fiber v2.52.4                   │
 │               http://127.0.0.1:8080               │
 │       (bound on host 0.0.0.0 and port 8080)       │
 │                                                   │
 │ Handlers ............ 534 Processes ........... 1 │
 │ Prefork ....... Disabled  PID ............. 22856 │
 └───────────────────────────────────────────────────┘

Disclaimer

  • I agree
@H0llyW00dzZ
Copy link
Contributor Author

Additional note

Regarding the philosophy behind this Feature Request, it's inspired by the Unix philosophy of writing programs that do one thing and do it well. By refactoring the Redis connection and addressing the EOF error, we are ensuring that this specific component of the application is functioning correctly and reliably, adhering to the principles of modularity and simplicity.

@Ujstor
Copy link
Collaborator

Ujstor commented May 1, 2024

@H0llyW00dzZ
I think you can implement this one together with #230

I propose that you close #227, as I see you have already implemented TLS here. That way, we can cover the complete Redis config in one PR and one release

@H0llyW00dzZ
Copy link
Contributor Author

@H0llyW00dzZ I think you can implement this one together with #230

I propose that you close #227, as I see you have already implemented TLS here. That way, we can cover the complete Redis config in one PR and one release

Later -> #230 (comment).

Currently, I am still testing the performance of using multiple databases in production, specifically MySQL and Redis. So far, it has been stable.

Has been up 17 days, 20 hours, 4 minutes, 44 seconds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants