perlundq/yajsync

--bwlimit and external project dependency

Opened this issue · 2 comments

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?

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.

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.