Skip to content
This repository was archived by the owner on Apr 30, 2024. It is now read-only.

Conversation

@tannermiller-wf
Copy link
Contributor

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.

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.
@tannermiller-wf
Copy link
Contributor Author

@robertkluin-wf @beaulyddon-wf @ericolson-wf @tylertreat-wf

@ericolson-wf
Copy link
Contributor

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()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ghost
Copy link

ghost commented Oct 8, 2013

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 get_result call in, how does this compare?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants