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

Configurable SLEEP_DURATION #13

Closed
lso400 opened this issue Aug 12, 2024 · 9 comments
Closed

Configurable SLEEP_DURATION #13

lso400 opened this issue Aug 12, 2024 · 9 comments

Comments

@lso400
Copy link

lso400 commented Aug 12, 2024

The SLEEP_DURATION is hardcoded to one second, can we make that configurable?

@bocharov
Copy link
Collaborator

We could consider making SLEEP_DURATION configurable, but first I'd like to understand your use case a bit better.
Is current default of 1 second too much or too little? There is also grace_duration parameter of Synchronizer::write method, which is configurable and related to the same logic of preventing "dead readers" problem.

@lso400
Copy link
Author

lso400 commented Aug 13, 2024

We could consider making SLEEP_DURATION configurable, but first I'd like to understand your use case a bit better. Is current default of 1 second too much or too little? There is also grace_duration parameter of Synchronizer::write method, which is configurable and related to the same logic of preventing "dead readers" problem.

Hey thanks for looking into this. One second is too long for us. I think grace_duration does not solve the issue we're seeing
E.g. We set grace_duration to 100ms. When num_readers>0 but Instant::now() < grace_expiring_at, the thread will be blocked for 1 second. We want to block for say 10ms and check num_readers again.

@bocharov
Copy link
Collaborator

Thanks for explaining your use case in more detail. I think we can definitely make this configurable - would you prefer it as template parameter on Synchronizer struct, which we can configure to default 1s (still configurable) and not break existing methods signature or perhaps new parameter to existing method or completely new method with new parameter?

@lso400
Copy link
Author

lso400 commented Aug 13, 2024

template parameter on Synchronizer struct, which we can configure to default 1s (still configurable)

This sounds good to me!

@bocharov
Copy link
Collaborator

Perhaps, other alternative is to add tokio optional feature, which will use tokio::time::sleep instead of std::thread::sleep, which doesn't block the thread during the sleep and allows other workload to progress.

@lso400
Copy link
Author

lso400 commented Aug 13, 2024

Perhaps, other alternative is to add tokio optional feature, which will use tokio::time::sleep instead of std::thread::sleep, which doesn't block the thread during the sleep and allows other workload to progress.

I'm afraid that this will not be helpful to applications running in sync runtime. IMO making it a template parameter on Synchronizer struct makes most sense. But of course tokio optional feature can be added on top to avoid blocking the entire thread for async runtime app

@bocharov
Copy link
Collaborator

Sure, I'll make template parameter change and release new package version tomorrow.

@bocharov
Copy link
Collaborator

@lso400 change now has been implemented and released as package version 1.0.4 via #14

@lso400
Copy link
Author

lso400 commented Aug 14, 2024

Thanks for the prompt action, much appreciated!

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

No branches or pull requests

2 participants