robfig / cron

a cron library for go
MIT License
13.09k stars 1.62k forks source link

Multiple Cron jobs running #425

Open monikanaico-hub opened 3 years ago

monikanaico-hub commented 3 years ago

HI First I used V1 in my code. I write my code inside main function. I am creating Rest APIs 1st I created a cron job with a specific time.. then that works perfectly. Then I required to change the time of cronjob, Then a changed the schedule and build added. after that 2 cronjobs are running.. Then i changed to version V3. But currently 2 cronjobs are running. How can I solve this

func NotifyWeeklyActivity(db *sql.DB, logger *zap.Logger) {
    logger.Info("NotifyWeeklyActivity- start")
    c := cron.New()
    c.AddFunc("*/5 * * * *", func() {
        lastweekTime := CurrentUTC().AddDate(0, 0, -7)

        type PostCount struct {
            HiveID               uint64      `json:"hive_id"`
            Post                 uint64      `json:"post"`
            NotificationTopicArn null.String `json:"notification_topic_arn"`
        }
        var posts []PostCount
        err := queries.Raw(`
            select count(post_id) as post , post.hive_id as hive_id , hive.notification_topic_arn
            from post
            join hive on post.hive_id=hive.hive_id and hive.deleted_at is null
            where post.deleted_at is null
            and hive.deleted_at is null
            and post.created_at between ? and ?
            group by hive_id
            having count(post_id)>3 ;
    `, lastweekTime, CurrentUTC()).Bind(context.TODO(), db, &posts)

        if err != nil {
            logger.Error("error while fetching data ", zap.Error(err))
            // return err
        }
        cfg, _ := config.GetImpart()
        if cfg.Env != config.Local {
            notification := NewImpartNotificationService(db, string(cfg.Env), cfg.Region, cfg.IOSNotificationARN, logger)
            logger.Info("Notification- fetching complted")
            for _, hive := range posts {
                pushNotification := Alert{
                    Title: aws.String(title),
                    Body: aws.String(
                        fmt.Sprintf("Check out %d new posts in your Hive this week", hive.Post),
                    ),
                }
                additionalData := NotificationData{
                    EventDatetime: CurrentUTC(),
                    HiveID:        hive.HiveID,
                }
                Logger.Info("Notification",
                    zap.Any("pushNotification", pushNotification),
                    zap.Any("additionalData", additionalData),
                    zap.Any("hive", hive),
                )
                err = notification.NotifyTopic(context.Background(), additionalData, pushNotification, hive.NotificationTopicArn.String)
                if err != nil {
                    logger.Error("error sending notification to topic", zap.Error(err))
                }

            }
        }
    })
    c.Start()
}

first I set time as /1 Now its changed to "/5 ".. But now i am getting 2 notification in each 5 mints.. How can I solve this.. How can I stop the existing cronjob?

G2G2G2G commented 3 years ago

Not sure what all the version change talk above has to do with anything.

I guess judging by how your code is formatted, somewhere you call NotifyWeeklyActivity twice and so it is running two separate crons entirely, you should have ONE function for cron, that runs scheduled cron jobs.. not running it inside of different functions that get called randomly. (see second example)

I assume you're saying you want that to run only one at a time, you can get a variable outside of the function that is like "running" to false and set it to true when the script starts and false when it ends..

like for yours I'd do

isRunning := false

logger.Info("NotifyWeeklyActivity- start")   //why is this here instead of inside the func?

c := cron.New()
c.AddFunc("*/5 * * * *", func() {
    //close out of func if it is already running
    if isRunning {
        return
    }
    isRunning = true

    //do all the rest of your function stuff
   isRunning = false
})
c.Start()

I'd restyle yours more like:


func mainCron() {
    //all cronjobs here
    c := cron.New()
    c.AddFunc("* * * * *", blahblah)
    c.AddFunc("* * * * *", someotherone)

    isNotifyWeeklyActivityRunning := false
    c.AddFunc("*/5 * * * *", NotifyWeeklyActivity)
}

func blahblah() {
}
func someotherone() {
}

func NotifyWeeklyActivity(db *sql.DB, logger *zap.Logger) {
    if isNotifyWeeklyActivityRunning {
        //close function if it's already running
        return
    }
    isNotifyWeeklyActivityRunning = true

    //see now it actually tells you when this runs
    logger.Info("NotifyWeeklyActivity- start") 

    lastweekTime := CurrentUTC().AddDate(0, 0, -7)

        type PostCount struct {
            HiveID               uint64      `json:"hive_id"`
            Post                 uint64      `json:"post"`
            NotificationTopicArn null.String `json:"notification_topic_arn"`
        }
        var posts []PostCount
        err := queries.Raw(`
            select count(post_id) as post , post.hive_id as hive_id , hive.notification_topic_arn
            from post
            join hive on post.hive_id=hive.hive_id and hive.deleted_at is null
            where post.deleted_at is null
            and hive.deleted_at is null
            and post.created_at between ? and ?
            group by hive_id
            having count(post_id)>3 ;
        `, lastweekTime, CurrentUTC()).Bind(context.TODO(), db, &posts)

        if err != nil {
            logger.Error("error while fetching data ", zap.Error(err))
            // return err
        }
        cfg, _ := config.GetImpart()
        if cfg.Env != config.Local {
            notification := NewImpartNotificationService(db, string(cfg.Env), cfg.Region, cfg.IOSNotificationARN, logger)
            logger.Info("Notification- fetching complted")
            for _, hive := range posts {
                pushNotification := Alert{
                    Title: aws.String(title),
                    Body: aws.String(
                        fmt.Sprintf("Check out %d new posts in your Hive this week", hive.Post),
                    ),
                }
                additionalData := NotificationData{
                    EventDatetime: CurrentUTC(),
                    HiveID:        hive.HiveID,
                }
                Logger.Info("Notification",
                    zap.Any("pushNotification", pushNotification),
                    zap.Any("additionalData", additionalData),
                    zap.Any("hive", hive),
                )
                err = notification.NotifyTopic(context.Background(), additionalData, pushNotification, hive.NotificationTopicArn.String)
                if err != nil {
                    logger.Error("error sending notification to topic", zap.Error(err))
                }

            }
        }

    isNotifyWeeklyActivityRunning = false
}

something like that

G2G2G2G commented 3 years ago

alternatively I guess you could also use https://tour.golang.org/concurrency/9 ?

monikanaico-hub commented 3 years ago

@G2G2G2G Thank you for your support.. I got the Issue.. Issue due to multiple instance is running in AWS