Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding FuturesOrdered::prepend #2309

Closed
benbromhead opened this issue Jan 12, 2021 · 4 comments · Fixed by #2591
Closed

Consider adding FuturesOrdered::prepend #2309

benbromhead opened this issue Jan 12, 2021 · 4 comments · Fixed by #2591
Labels
A-stream Area: futures::stream C-feature-request

Comments

@benbromhead
Copy link

What:
Consider adding a method to FuturesOrdered that allows adding a future to the front of the task queue.

Why:
FuturesOrdered is useful for completing tasks in order, however handling futures that error out or need to be retried can be messy. By creating a prepend method, we can add new futures that retry a failed task or job to the front of the queue within a single loop.

PR:
I haven't created a PR yet, but I've created a naive example implementation here: benbromhead@ba30579

E.g.

let mut fo = FuturesOrdered::new();
fo.push(my_task(1));
fo.push(my_task(2));
fo.push(my_task(3));
fo.push(my_task(4));
fo.push(my_task(3));
fo.push(my_task(2));
fo.push(my_task(1));

let mut results: Vec<i32> = vec![];

while let Some(result) = fo.next().await {
    match result {
        Ok(number) => {
            // handle success
        }
        Err(_) => {
            //oh no, let's retry this
            fo.prepend(my_task(3))
        }
    }
}

Notes:

  • I don't think the name prepend is 100% right, but the felt right at the time.
  • Does this need an RFC? Felt simple, but I wasn't sure.
@taiki-e taiki-e added A-stream Area: futures::stream C-feature-request labels Jan 12, 2021
@taiki-e
Copy link
Member

taiki-e commented Jan 14, 2021

I think it is fine with adding a method that adds the future in a different position than push, but I'm not clear what kind of API is preferable.

I've created a naive example implementation here: benbromhead@ba30579

At first glance, that implementation would panic if prepend was used with an empty FuturesOrdered.

  • Does this need an RFC? Felt simple, but I wasn't sure.

No RFC needed. (futures-rs does not currently use a mechanism like RFC.)

@benbromhead
Copy link
Author

Updated the example: https://github.com/benbromhead/futures-rs/blob/2a9ebf5cfa0e076af58f6c5e46aa518a8538e6ab/futures-util/src/stream/futures_ordered.rs#L157-L169

I'll also update with some tests shortly and get it ready for a PR if you are happy with this approach / terminology

@marcospb19
Copy link
Contributor

Was searching for this exact issue, I'd suggest push_front for the same reason, some Futures need to be retried, my current solution is to have an additional Option in my struct that keeps the element to retry, instead of adding it back to the FuturesOrdered.

@conorbros
Copy link
Contributor

This can be closed as #2591 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stream Area: futures::stream C-feature-request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants