taylorchu / work

gocraft/work v2 prototype
MIT License
147 stars 23 forks source link

Some features requested #16

Closed paulm17 closed 4 years ago

paulm17 commented 4 years ago

I modded the original gocraft/work a bit to suit my needs and would really like to see these natively:

1) Have completed callback for when a job successfully completes. 2) Have a failed callback for when a job fails and has retries left and have available a stack trace. 3) Have a completely failed callback for when a job fails and has no retries left.

I got so far with a test, but without a context I'm unable to proceed and start testing. I await future updates with interest! 👍

Many thanks

taylorchu commented 4 years ago

this can be done with middleware. here is one example that looks at whether a job fails or not: https://github.com/taylorchu/work/blob/db6aea54d2ef643f1644bea0071f824342c9b625/middleware/logrus/logger.go#L11

you can use job.Retries https://github.com/taylorchu/work/blob/db6aea54d2ef643f1644bea0071f824342c9b625/job.go#L30 to examine failed count in your middleware.

paulm17 commented 4 years ago

Awesome, thank you for the quick feedback.

Any timeline on the context issue?

taylorchu commented 4 years ago

maybe next week. It will add a deadline specified in job options.

we actually don't use context that much internally. it is a nice-to-have if the job content code supports cancellation. how do you plan to use context?

paulm17 commented 4 years ago

Here's how I'm using the context

type context struct {
    DB          *sql.DB
    ClientID    string
    Config      common.TomlConfig
}

func main() {
// Get Config
currentDir := common.GetDir()
config := common.ReadConfig(currentDir + "/config.toml")
db, err := model.MySQLConnect(config)
redisPool := model.RedisConnect(config)

if err != nil {
    fmt.Println("Cannot connect to MySQL Database")
    os.Exit(0)
}

env := &worker.Env{
    DB:        db,
    RedisPool: redisPool,
}

// Get Nats instance
sc, sub, clientID := env.Start(config)

// Make a new pool. Arguments:
// context{} is a struct that will be the context for the request.
// 10 is the max concurrency
// "my_app_namespace" is the Redis namespace
// redisPool is a Redis pool
pool := work.NewWorkerPool(context{}, 10, "ns1", redisPool)

// Add middleware to pass the database connection to the worker
// functions below.  Ensure this is first.
pool.Middleware(func(c *context, job *work.Job, next work.NextMiddlewareFunc) error {
    c.DB = db
    c.ClientID = clientID
    c.Config = config

    return next()
})

// Map the name of jobs to handler functions
pool.Job(config.Nats.Queue, (*context).damInit)

...

// damInit function to resize images or generate thumbs
func (c *context) damInit(job *work.Job) error {    

The c.DB is a database connection to a memsql cluster. The c.ClientID is the ID of every worker that is initiated and there is a list maintained on the primary job server. The c.Config is a configuration for other services that the worker can then choose to execute down stream.

So for me context is a bit critical.

Hope that helps.

taylorchu commented 4 years ago

will context.Context be useful for you?

paulm17 commented 4 years ago

Golang isn't a daily driver, so I don't know the API as well as something like javascript.

Is it an interface which each worker can inherit by a middleware mutation, much like gocraft/work?

taylorchu commented 4 years ago

no, it is from: https://golang.org/pkg/context/. this is supported by wider range of library as the first argument of a function or method call.

paulm17 commented 4 years ago

I miss understood. Sorry about that.

I also thought context was under the original paradigm and just by trying to figure it out myself, I have it working. Should have done that earlier.

Here's an example:

type Context struct {
    Text string
}

 func main() {
         var c Context
         c.Text = "GOAT"

        err := w.Register("worker",
        func(job *work.Job, _ *work.DequeueOptions) error {
        err := doWork(c)

        return err
        },

        ...

func doWork(c Context) error {
    fmt.Println(c)

    return nil
}

This solves my usecase for each worker inheriting from main.

I'll close this once I complete the 3 requirements above. I'll look into that shortly.

paulm17 commented 4 years ago

I'm closing this as I have satisfied my requirements for moving over from gocraft/work to this new implementation.

I've only done a few test, but I am amazed at how much faster it is compared to the old one! 🥇

It's also a much easier library to grok with, so I am really happy about that.

Looking forward to what else comes next.

Again, many thanks for building this library! 👍