search

tarantool / queue

Create task queues, add and take jobs, monitor failed tasks
Other
237 stars 52 forks source link

Add an ability to customize task options (and data) when calling tube:release() #125

Open rybakit opened 4 years ago

rybakit commented 4 years ago

tube:release() accepts an optional second argument opts, which at this moment can only consist of delay. But often when I release a task back into the queue, I need to adjust other task options, such as priority or ttl. Also, in task data I usually store additional options such as retry strategy, retry count, etc, and it would be great if I can adjust them as well when calling tube:release() (or even better, add an extra field called extra or meta where users can store custom task options?).

LeonidVas commented 3 years ago

What do you think about implementing the task "options" update in a separate method like touch?

rybakit commented 3 years ago

What do you think about implementing the task "options" update in a separate method like touch?

@LeonidVas Could you elaborate on how you see it, what options will it support? Maybe you have some examples on hand?

LeonidVas commented 3 years ago

What do you think about implementing the task "options" update in a separate method like touch?

@LeonidVas Could you elaborate on how you see it, what options will it support? Maybe you have some examples on hand?

Respond to question with counter-questions - not bad)(Вы, таки, не с Одессы будете?) Frankly, I started thinking about this task after you refer to it in https://github.com/tarantool/queue/pull/149 . Now I can only share my raw thoughts on this theme:

For ascertaining the answers, I should to look around and see how this works in other similar software, and when I asked you a question, I thought you have some knowledge in this area and you would like to share it.

rybakit commented 3 years ago

OK, let me explain then why I asked you about giving more details :) When you proposed the new api method, I thought you had already thought about how it would work and what it should accept. For example, will it be able to increment, decrement, or reset tasks ttr:

t:options(task_id, {ttr_inc=60})

Will it allow to change the task state:

t:options(task_id, {state=state.READY})

or/and delay:

t:options(task_id, {delay=42})

If so, what do you plan to do with the existing functions that already have this functionality? Deprecate them or keep them? If not, what options do you actually propose to allow with this new function? Also, what about the custom options that I mentioned in the description? Will it support them and in which form?

See, there are tons of questions and just throwing random ideas without specifying exactly what you meant doesn't help, no one can read your mind. Without knowing these details, I can't give any meaningful answer. But if you're expecting a super generic answer: no, I don't really like it, and I don't remember seeing anything like that in any other related product. At a glance, it feels too low-level end error-prone, because you can't make those option arguments part of the interface (if this term can be applied to lua :)). Also, I don't know what problem you are trying to solve by introducing a new function. If it's about keeping release() from getting too bloated, maybe it's better to introduce release_with_opts()? So many questions :D

rybakit commented 3 years ago

Now I can only share my raw thoughts on this theme:

Thanks for that! (I wish you did it from the beginning :))

I don't know if this is a true way to change input parameters of a task (although we already have the touch method that can affect ttl).

Neither do I. Please see my thoughts below.

release called by the worker and IMHO it is outside of his competence to change the input parameters of a task.

Good point. And I don't have a clear answer for that. It could depend on how we treat these input parameters. Of course, if they were "domain" parameters like user_id or deposit_amount I would fully agree with you. But when I look at the list of currently available task options, they are all worker-specific, and I don't see how they can be used elsewhere. So maybe it's more about user responsibility and awareness that such options can be modified by the worker? Or maybe they can be guarded with a "read-only" flag or something to prevent "implicit" modification, Idk :)

Now we already have a method which can affect the input parameters of a task(touch), and maybe it would be logical to expand functionality of this method instead of adding such functionality to the release method.

From the usability perspective, if I had to choose between

t:touch(task_id, 60);

and 

t:options(task_id, {ttl={'+', 60}})

I would choose the former just because it reads better and I don't have to check the option name and how to increment its value in the docs. On the other hand, having the ability to modify multiple options at once could be useful when used from synchronous connectors (see https://github.com/tarantool/queue/issues/124).

FTR, If you decide to go with the options() approach, it will not solve my original issue, as I would have to call the two functions together all the time:

t:release(task_id)
t:options(task_id, {ttr={'+', 60}, prio={'+', 1}, meta={ retry={'+', 1} }})

which means that I have to extend this library by adding my own Lua function that wraps these calls. This completely defeats the purpose of this ticket, as I already have such a wrapper that I would like to get rid of.

After all, I'm more in favor of extending release() than adding options(). I don't think it's worth introducing a new function because it would require deprecating the second argument of release() and touch(). But if you decide to keep it, it would mean duplicating functionality which is another source of confusion for users. And as I already mentioned, I simply don't have a use case where I would use options() independently of release() (and btw, I don't have a use case for touch() either).