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

Use max_items as bound in tower-batch #1691

Merged
merged 2 commits into from
Feb 5, 2021

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Feb 4, 2021

Motivation

We want to change the max number of requests of the semaphore in zebra-batch.

Closes #1670

Solution

I discussed with @teor2345 that the best by now for this will be to use max_items argument as bound.

Review

@teor2345

@oxarbitrage
Copy link
Contributor Author

oxarbitrage commented Feb 4, 2021

I am wondering if we need:

https://github.com/tower-rs/tower/blob/master/tower/src/semaphore.rs#L93-L100

If so, we might have to use an older version:

tower-rs/tower@43c4492#diff-962111ba9c46d1673d59e205b3c1a9ff6996e90e23863e91a192d148e29512bc

We changed the approach on how to do this, this comment do not apply anymore.

@oxarbitrage oxarbitrage closed this Feb 4, 2021
@oxarbitrage oxarbitrage reopened this Feb 4, 2021
@oxarbitrage oxarbitrage changed the title Port bound from tower-buffer into Zebra tower-batch Use use max_items as bound in tower-batch Feb 4, 2021
tower-batch/src/service.rs Outdated Show resolved Hide resolved
Co-authored-by: teor <[email protected]>
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

We're good here

@teor2345 teor2345 merged commit bfb3de7 into ZcashFoundation:main Feb 5, 2021
@teor2345 teor2345 changed the title Use use max_items as bound in tower-batch Use max_items as bound in tower-batch Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the correct bound for the semaphore in tower-batch
2 participants