go-co-op / gocron

Easy and fluent Go cron scheduling. This is a fork from https://github.com/jasonlvhit/gocron
MIT License
5.68k stars 309 forks source link

[BUG] - Distributed lock not working correctly #799

Open OrkhanAlikhanov opened 1 week ago

OrkhanAlikhanov commented 1 week ago

Describe the bug

When using a redis distributed locking, I still see the same task being run concurrently instead of one at a time.

To Reproduce

When I run the below code, I expect to see single task being run at a time because of the locking:

[54dd288b-7f4a-46b1-8cc8-443d20e72712] starting 
[54dd288b-7f4a-46b1-8cc8-443d20e72712] running
[54dd288b-7f4a-46b1-8cc8-443d20e72712] running
[54dd288b-7f4a-46b1-8cc8-443d20e72712] running
[54dd288b-7f4a-46b1-8cc8-443d20e72712] running
[54dd288b-7f4a-46b1-8cc8-443d20e72712] finished
[7ea08919-dddb-441c-9853-5322c5c6b367] starting 
[7ea08919-dddb-441c-9853-5322c5c6b367] running
[7ea08919-dddb-441c-9853-5322c5c6b367] running
[7ea08919-dddb-441c-9853-5322c5c6b367] running
[7ea08919-dddb-441c-9853-5322c5c6b367] running
[7ea08919-dddb-441c-9853-5322c5c6b367] finished
[a8832919-cae7-4cbc-8681-e822d918b42d] starting 
[a8832919-cae7-4cbc-8681-e822d918b42d] running

However, I see:

[007dddfd-e2e6-47c6-bb81-4186281d3cce] starting 
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] starting 
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] starting 
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] running
[1f64a44d-8592-4106-afae-029ab0866fc2] starting 
[1f64a44d-8592-4106-afae-029ab0866fc2] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
[007dddfd-e2e6-47c6-bb81-4186281d3cce] finished
[1f64a44d-8592-4106-afae-029ab0866fc2] running
[ac0c1f59-4e34-4f30-bb56-0a411b17a167] running
[61997a91-b0b0-4cd7-91a4-d55bd6d5b8aa] running
package main

import (
    "context"
    "fmt"
    "os"
    "os/signal"
    "syscall"
    "time"

    "github.com/go-co-op/gocron/v2"
    "github.com/google/uuid"

    "github.com/redis/go-redis/v9"

    redislock "github.com/go-co-op/gocron-redis-lock/v2"
)

func main() {
    ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
    defer cancel()

    // create a scheduler
    s, err := gocron.NewScheduler(
        gocron.WithLogger(gocron.NewLogger(gocron.LogLevelDebug)),
        gocron.WithDistributedLocker(getLocker()), // use redis locker
    )
    if err != nil {
        panic(err)
    }

    // add a job to the scheduler
    _, err = s.NewJob(
        gocron.DurationJob(
            1*time.Second,
        ),
        gocron.NewTask(func() {
            id := uuid.New().String()
            fmt.Printf("[%s] starting \n", id)

            ctx, cancel := context.WithTimeout(ctx, 25*time.Second)
            defer cancel()

            // start logging
            for {
                select {
                case <-ctx.Done():
                    fmt.Printf("[%s] finished\n", id)
                    return
                default:
                    fmt.Printf("[%s] running\n", id)
                    time.Sleep(1 * time.Second)
                }
            }
        }),
    )
    if err != nil {
        panic(err)
    }

    // start the scheduler
    s.Start()

    // wait until done
    <-ctx.Done()

    // shut it down
    err = s.Shutdown()
    if err != nil {
        panic(err)
    }
}

func getLocker() gocron.Locker {
    redisOptions := &redis.Options{
        Addr: "localhost:6379",
    }
    redisClient := redis.NewClient(redisOptions)
    locker, err := redislock.NewRedisLocker(redisClient, redislock.WithTries(1))
    if err != nil {
        panic(err)
    }

    return locker
}

Version

require (
    github.com/go-co-op/gocron-redis-lock/v2 v2.0.1
    github.com/go-co-op/gocron/v2 v2.12.3
)

Expected behavior

Single task being run at a time regardless of how long the task takes.

I skimmed the code, I do not see where the lock is extended. Since redis locks have expiration, they need to be extended before the expiration hits. This should happen as long as the task is running. I think the locker interface should have an Extend method and should call it at certain interval before the lock expires.