Using timeouts

[DavidMet]

Hi @oze4 , Hope you doing good!

I've tried the latest version and I saw that you made some changes to the timeout and add the contexts,

1. Could you please explain what is the use-case?
2. e.g. Two jobs with two different timeouts, am I do it right ? if so, dont you think it kind of boilerplate ? is there a way to make it shorter?

defaultCtx := context.Background()
numWorkers := 10
wp := wpxt.New(defaultCtx, numWorkers)
timeout := time.Duration(time.Millisecond)
timeout2 := time.Duration(time.Seconds)

myCtx, done := context.WithTimeout(context.Background(), timeout)
defer done()

wp.SubmitXT(wpxt.Job{
    Name: "Job1",
    Context: myCtx,
    Task: func(o wpxt.Options) wpxt.Response {

        time.Sleep(time.Second*10) 
        return wpxt.Response{Data: "I could be anything"}
    },
})


myCtx2, done2:= context.WithTimeout(context.Background(), timeout2)
defer done2()

wp.SubmitXT(wpxt.Job{
    Name: "Job2",
    Context: myCtx2,
    Task: func(o wpxt.Options) wpxt.Response {
        return wpxt.Response{Data: "I could be anything"}
    },
})

3.Btw, when you use the defer done() the job doesnt wait to the timeout, it finish immeditally, should I use it like this? or I can remove the defer and use it like myCtx, _ := context.WithTimeout(context.Background(), timeout) ?

Thanks a lot!

[oze4]

Hey @DavidMet hope you are well also!

While I am still learning about context myself, using context allows the user more control than just timeouts. You now have the ability to force cancel a job even before the timeout.

I asked the author of workerpool for his feedback on this library and he explained to me why using context is the better route. I do agree it is a little more boilerplate but at the end of the day it's worth it. You can see his input in #7

As far as defer not working, in my tests it has been working so that may be a new bug. I am still cleaning things up internally and should have updated code pushed today.

[oze4]

@DavidMet I just published v1.1.1 if you would like to take a look/test it out that would be awesome!

[oze4]

It also looks like defer is not why your job is timing out immediately. It's because you're telling it to timeout immediately..

// Taken from your code example above

// ...
// this means the job will timeout in 1ms
timeout := time.Duration(time.Millisecond)
// ...

[oze4]

I also added this test so we can check for this type of thing.

workerpoolxt/workerpoolxt_test.go

Line 607 in 261e230

func TestDeferDoesNotCancelJobImmediately(t *testing.T) {

[oze4]

oze4 added a commit that referenced this issue 3 hours ago

Sorry meant to reference #9 instead

[oze4]

Also, we do create a child context from the context you've provided. You should be able to just exclude the cancel func if you don't need to call it.

[DavidMet]

@oze4 - well, I understand the situation where it's valuable to add the context, my question now, do you think there is a way to simplify the usage of it , timeout with context ...,if so, could you please update the docs..
Thanks for you hard work this is really useful package!

[oze4]

@DavidMet you could write a func that returns a new context with timeout based upon a parameter. Then call that func in each jobs Context.

This is not the responsibility of this library to enforce or document how people create context.

For example, in my tests I am using a func to create the context. You can do the something like this but use a parameter to pass the timeout to context.WithTimeout.

workerpoolxt/workerpoolxt_test.go

Line 24 in 80320f8

freshCtx = func() context.Context { return context.Background() }

workerpoolxt/workerpoolxt_test.go

Line 38 in 80320f8

w := New(freshCtx(), numWorkers)

[oze4]

@DavidMet I was finally able to get to my computer...

Something like this is what I mean...

func newContextWithTimeout(timeout time.Duration) context.Context {
	r, _ := context.WithTimeout(context.Background(), timeout)
	return r
}

func main() {
	defaultContext := context.Background()
	wp := wpxt.New(defaultContext, 10)
	
	wp.SubmitXT(wpxt.Job{
		Name: "a",
                // Set timeout to whatever for each job....
		Context: newContextWithTimeout(time.Duration(time.Second*100)),
		Task: // ...
	})
	
	wp.SubmitXT(wpxt.Job{
		Name: "a",
		Context: newContextWithTimeout(time.Duration(time.Millisecond*5)),
		Task: // ...
	})
	
	// ...
}

[DavidMet]

yes. what about the chancel ,should I use it somewhere ?

r, _ := context.WithTimeout(context.Background(), timeout)here you dont use it ...
it get also the CancelFunc , can you please clarify ? what could be the implications ...