desync icon indicating copy to clipboard operation
desync copied to clipboard

Add draft of throttled copy

Open bearrito opened this issue 2 years ago • 4 comments

Adds ability to do throttled copy operations related to filling the cache. Problem Basically I'm trying to replicate what curl offers - https://everything.curl.dev/usingcurl/transfers/rate-limiting

We had discussed before doing this store/server side. I'm not sure that works for our case.

  • We have many devices 500-1500 per facility (warehouse)

  • Each device is generally doing work, so we need to dribble in updates in the background

  • Devices are wifi aware so if they are in a better part of the facility they can stream faster.

  • The store wouldn't have knowledge of IP to device. They store wouldn't know if the device is busy or in a good wifi area.

Approach I think I took a pretty standard approach for rate-limiting workers. I do think it would have been better if I was able to get the size of the chunks. Right now waiting a fixed period essentially just limits the number of chunks per second, which will get your roughly (Average Chunk Size ) / second of bytes. At least with this way there is no coordination between workers in summing up bytes.

bearrito avatar Mar 03 '24 16:03 bearrito

Having this feature is a good idea, it could be quite useful. But I think this could be implemented a bit more generically.

How about:

  • Making the Limiter into a wrapper for a "Store" and "WriteStore". Then you wouldn't need to make any changes to the Copy function at all, the store passed into it would be rate-limited
  • In the Limiter, use https://pkg.go.dev/golang.org/x/time/rate to do the actual limiting rather than a custom sleep. This would also allow you to limit based on the number of bytes in the chunk. Probably have to make sure that the burst size is equal/bigger than than max-chunk-size
  • Could add a separate commandline flag for write-limit and read-limit in bytes per second

It'd be nice if we could make that more generic than just the copy function and expand to all, though it might be hard to specify this on the commandline since there could be several stores involved in a single command. Perhaps we could just expect this to be defined in store config file (https://github.com/folbricht/desync?tab=readme-ov-file#configuration).

Thoughts?

folbricht avatar Mar 04 '24 11:03 folbricht

No problem. I'll obviously defer to your expertise in the matter. I'll have to look at the code-base more, but I guess I need to figure out how to provide backpressure from the write store.

Edit: Are you good with me limiting this to the LocalStore first?

bearrito avatar Mar 04 '24 14:03 bearrito

You don't need to change the LocalStore at all. You'll just make a wrapper for any kind of store. The steps would be roughly:

  • Make a new struct like RateLimitStore or similar (in a new file)
  • The struct should have at least these methods https://github.com/folbricht/desync/blob/master/store.go#L16-L20 to implement the Store interface. For a good example take a look at swapstore.go, it's very close to what you need. Of course yours wouldn't be about swapping the underlying store, but limiting the GetChunk() method. Initially you could just pass the methods through to the wrapped store.
  • Your struct would have a constructor like this func NewRateLimitStore(s Store) *RateLimitStore {
  • Take a look at how the source-store is intitialized in https://github.com/folbricht/desync/blob/master/cmd/desync/cache.go#L103-L107, after that line you'd do something like s = NewRateLimitStore(s) which would wrap whatever the read store is. We can later take a look at how to generalize this more if needed, like if we want to be able to limit the writing part as well.

Hope that helps, let me know if not and I can whip up a template on a branch which you can then complete.

folbricht avatar Mar 05 '24 12:03 folbricht

Add throttle on reads.

bearrito avatar Mar 10 '24 19:03 bearrito