Change _insert_tasks to use add_async #112
Conversation
In _insert_tasks(), we now add each task individually using Queue.add_async(), instead of all at once with Queue.add(). This allows us to insert each task only once and not have to worry about splitting and retrying until we find the bad tasks. Unfortunately this method prevents us from determining exactly how many tasks were successfully inserted. This will help however when a large number of duplicated tasks are trying to be added and it keeps splitting instead of just quitting.
|
@robertkluin-wf @beaulyddon-wf @ericolson-wf @tylertreat-wf |
|
This looks pretty good. I assume this helps speedup some of our worst cases. Do we want to make this optional? I'd hope we wouldn't need to make it optional if this works well. Does the developer sometimes want to ensure the tasks have been inserted? Maybe storing the futures on the class would allow a developer to make get_result calls? - I'm not sure if keeping those handles would use more memory. Also, any memory bloat by using many single add_async() instead of one group add()? |
| @@ -247,21 +247,22 @@ def _insert_tasks(tasks, queue, transactional=False): | |||
| if not tasks: | |||
| return 0 | |||
|
|
|||
| try: | |||
| taskqueue.Queue(name=queue).add(tasks, transactional=transactional) | |||
There was a problem hiding this comment.
I'd like to see more statistics on this. Just some basic scenario testing....
Old style (batch and split)
All Async
Batch and Async (take X tasks and split into Y batches and insert that batches async)
Batch and Async on failure (Do the batch insert and if it fails then fallback to all async)
Batch and Async with Async on Failure (take X tasks and split into Y batches and insert that batches async and if those fail split into single tasks to insert async)
Then run those scenarios against 1, 10, 100, 1000 tasks, etc. Also run with some tasks failing and hitting the different split scenarios.
There was a problem hiding this comment.
|
As @beaulyddon-wf noted, you need to run some more tests. Specifically I would like to see "normal" case tests, meaning there are no splits, just a single successful batch insert. I would also like to see how the async-per-task compares with one, ten, one hundred, one thousand tasks. If you add the |
In
_insert_tasks(), we now add each task individually usingQueue.add_async(), instead of all at once withQueue.add(). This allowsus to insert each task only once and not have to worry about splitting
and retrying until we find the bad tasks. Unfortunately this method
prevents us from determining exactly how many tasks were successfully
inserted. This will help however when a large number of duplicated tasks
are trying to be added and it keeps splitting instead of just quitting.