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

--bwlimit and external project dependency #46

Open
usrflo opened this issue Apr 10, 2016 · 2 comments
Open

--bwlimit and external project dependency #46

usrflo opened this issue Apr 10, 2016 · 2 comments

Comments

@usrflo
Copy link
Contributor

usrflo commented Apr 10, 2016

In native rsync, io.c, the sleep_for_bwlimit function is commented as follows:

/* Sleep after writing to limit I/O bandwidth usage.
 *
 * @todo Rather than sleeping after each write, it might be better to
 * use some kind of averaging.  The current algorithm seems to always
 * use a bit less bandwidth than specified, because it doesn't make up
 * for slow periods.  But arguably this is a feature.  In addition, we
 * ought to take the time used to write the data into account.
 *
 * During some phases of big transfers (file FOO is uptodate) this is
 * called with a small bytes_written every time.  As the kernel has to
 * round small waits up to guarantee that we actually wait at least the
 * requested number of microseconds, this can become grossly inaccurate.
 * We therefore keep track of the bytes we've written over time and only
 * sleep when the accumulated delay is at least 1 tenth of a second. */

When adding --bwlimit to yajsync the mentioned improvement of the @todo section above (averaging) should be considered. The token bucket algorithm would fit the requirements of such an improvement.

Taking a look at https://github.com/bbeck/token-bucket I guess this would be an appropriate implementation to simply integrate bandwidth limits. Taking in account you like to keep dependencies low:

  • would you propose to re-implement such an algorithm?
  • would you propose to re-use existing code but integrate it with its license to the yajsync source code as long as being compatible?
  • would you be open for a maven dependency?
@perlundq
Copy link
Owner

I would not like to add that as an external dependency for such a simple thing. If this becomes a problem, then it might be added later (but unlikely, it's not that important to be that exact with the limit). So I would like to have the simple solution for this.

@usrflo
Copy link
Contributor Author

usrflo commented Dec 26, 2016

I modified https://github.com/bbeck/token-bucket to remove any dependency on https://github.com/google/guava, see token-bucket-wo-dependencies.zip.

Would it fit into the design of yajsync to add the resulting classes to a separate package like com.github.perlundq.yajsync.internal.util.ratelimit? I could rework the implementation and integrate everything into one class with multiple inner classes if you prefer a totally encapsulated implementation.

In case of bwlimit!=0 the token bucket would be instantiated as a singleton and used at low-level read/write operations in the channels.

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