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

Does not redislock work properly with multiple Go gin API at a time? #40

Closed
iamatsundere opened this issue Jan 9, 2022 · 4 comments
Closed

Comments

@iamatsundere
Copy link

iamatsundere commented Jan 9, 2022

Hi, I made an API that receive file

func main() {
	route := gin.Default()
	route.POST("/upload",func(c *gin.Context) {
	        //process file
		clientL := redis.NewClient(&redis.Options{
		                Network:  "tcp",
		                Addr:     "localhost:6379",
		                DB:       0,
	                })
	        defer clientL.Close()
	        locker := redislock.New(clientL)
	        ctx := context.Background()
	        lock, err := locker.Obtain(ctx, "file", 5000*time.Millisecond, nil)
	        if err != nil {
		        fmt.Println(err)
		        return
	        }
	        defer lock.Release(ctx)
                fmt.Println("pass")
                //return response
	})
	route.Run("0.0.0.0:8080")
}

Normally, I think when there are multiple requests at the same time want to upload a file. They must pause at the line I obtain the lock, if my lock is released or timeout, they will continue. It it doesn't, the request will fail. But when I make a client that sends 4 requests at the same time to my API, they can obtain the lock for all of them and it continues for next calls.

Did I do something wrong or I misunderstand something in this concept, please help!

@musclechen
Copy link

This redislock has no retry strategy by default. Your code looks like it will simply return false if the lock was not acquired. I don't quite understand what you said 'they can obtain the lock for all of them and it continues for next calls.' In theory, four threads grab the lock at the same time, and only one thread will succeed in grabbing the lock. I don't know if you are using postman. The four requests sent by postman at the same time are actually executed serially, that is, B will be executed after A is executed.
You can add one row like this after the 'Obtain'.
fmt.Print(lock) time.sleep(10*time.Second)'
If you can see four threads at the same time print the lock acquired successfully. May be a bug of redislock.

@iamatsundere
Copy link
Author

iamatsundere commented Jan 11, 2022

Yeah, sorry for the part I've talked about 'pausing'. What I mean is the lock would failed or process if it is obtainable or not. But whenever I call the API, my lock can be obtained. If I want to wait the lock to be obtained, I will use a loop to wait.

About 4 requests simultaneously, I am using a client written in ReactJS that makes multiple request at a time.

@dim
Copy link
Member

dim commented Jan 11, 2022

I don't know about gin but I just created a tiny HTTP server and it seems to be doing what it says on the tin :)

package main

import (
	"io"
	"net/http"
	"time"

	"github.com/bsm/redislock"
	"github.com/go-redis/redis/v8"
)

func main() {
	client := redis.NewClient(&redis.Options{
		Addr: ":6379",
		DB:   9,
	})
	defer client.Close()

	locker := redislock.New(client)
	http.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) {
		ctx := r.Context()
		lock, err := locker.Obtain(ctx, "file", 5*time.Second, nil)
		if err == redislock.ErrNotObtained {
			http.Error(rw, "Busy", http.StatusTooManyRequests)
			return
		} else if err != nil {
			http.Error(rw, err.Error(), http.StatusInternalServerError)
			return
		}
		defer lock.Release(ctx)

		select {
		case <-ctx.Done():
			http.Error(rw, "Gone", http.StatusGone)
			return
		case <-time.After(200 * time.Millisecond):
			// assume request takes 200ms to complete
		}

		io.WriteString(rw, "OK")
	})

	http.ListenAndServe(":8080", nil)
}

When I run the client code below, I get:

request #02: 429 "Busy"
request #03: 429 "Busy"
request #01: 200 "OK"
request #05: 429 "Busy"
request #06: 429 "Busy"
request #04: 200 "OK"
request #08: 429 "Busy"
request #09: 429 "Busy"
request #07: 200 "OK"
request #10: 200 "OK"
package main

import (
	"bytes"
	"fmt"
	"io"
	"log"
	"net/http"
	"sync"
	"time"
)

func main() {
	wg := sync.WaitGroup{}
	defer wg.Wait()

	for i := 0; i < 10; i++ {
		wg.Add(1)

		go func(n int) {
			defer wg.Done()

			time.Sleep(time.Duration(n) * 100 * time.Millisecond)

			resp, err := http.Get("http://localhost:8080")
			if err != nil {
				log.Fatalln(err)
			}

			b, _ := io.ReadAll(resp.Body)
			fmt.Printf("request #%02d: %d %q\n", n, resp.StatusCode, bytes.TrimSpace(b))
		}(i + 1)
	}
}

@dim dim closed this as completed Jan 11, 2022
@iamatsundere
Copy link
Author

Ok tks, the problem is in my code :)

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

No branches or pull requests

3 participants