search

junneyang / zumastor

Automatically exported from code.google.com/p/zumastor
0 stars 1 forks source link

support for replication bandwidth limit #91

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
zumastor replication can use a large amount of network bandwidth, so NFS
performance can be affected a lot during replication. We should add an
option to allow users to specify replication bandwidth limit.

Original issue reported on code.google.com by jiayin...@gmail.com on 25 Mar 2008 at 10:35

GoogleCodeExporter commented 9 years ago

Original comment by daniel.r...@gmail.com on 26 Mar 2008 at 6:22

GoogleCodeExporter commented 9 years ago 
Is network bandwidth or disk IO the bottleneck in this case?

Either way, I could see a use case for both.  If the data is compressing really 
well
like zeros do, we could still peg the disk with a network throttle.  Perhaps we
should provide two options (not necessarily mutually exclusive):

ddsnap <transmit|receive> ... [--throttle-net <rate>] [--throttle-io <rate>]

Original comment by sha...@gmail.com on 1 Apr 2008 at 10:10

GoogleCodeExporter commented 9 years ago 
In the test I was running, the network seems to be the bottleneck. There may be 
some
cases where snapshot read from replication has big impact on origin write
performance. I will do more experiments on this to be clear.

In the attachment is the patch that implements replication network throttling. 
Can
anyone review it? Thanks!

Original comment by jiayin...@gmail.com on 5 Apr 2008 at 12:44

Attachments:

GoogleCodeExporter commented 9 years ago 
The throttling patch looks good overall.  A few comments:
1) It looks like you deleted the entire ddsnap usage because it was out of 
date.  I
think it is useful for a quick reference.  You could just leave this alone and 
I can
clean it up with another patch if you want.
2) The manpage should be updated for both zumastor and ddsnap
3) You've added a pointless gettimeofday() system call for cases where neither
progress file nor ratelimit is used.  I suppose this is pretty minor because
gettimeofday is fast and zumastor always uses a progress file. *shrug*

Shapor

Original comment by sha...@gmail.com on 10 Apr 2008 at 5:36

GoogleCodeExporter commented 9 years ago 
Thanks for reviewing the patch!

    1) It looks like you deleted the entire ddsnap usage because it was out
    of date.  I
    think it is useful for a quick reference.  You could just leave this
    alone and I can
    clean it up with another patch if you want.

I changed the ddsnap usage printing to only print a short message.
The current ddsnap code prints detailed help message with 'ddsnap --help'.
but a short list of ddsnap commands with 'ddsnap --usage'. Every time
we add a command, we need to change both messages. I mentioned
this to DanP and he thinks maybe it is better that we just print a really
short message for 'ddsnap --usage' so that we don't need to maintain both.

This is separate from the network throttling. I took this change out
since you are object to it.

    2) The manpage should be updated for both zumastor and ddsnap

Updated them.

    3) You've added a pointless gettimeofday() system call for cases where neither
    progress file nor ratelimit is used.  I suppose this is pretty minor because
    gettimeofday is fast and zumastor always uses a progress file. *shrug*

I changed that part. I also made another small change by using nanosleep 
instead of
the obsolete usleep.

The new patch is attached. Could you take a look at it again? Thanks!

Jiaying

Original comment by jiayin...@gmail.com on 10 Apr 2008 at 7:24

Attachments:

GoogleCodeExporter commented 9 years ago 
There is now a problem if rate limiting isn't enabled current_time never gets 
updated
and the progress file won't either.

Original comment by sha...@gmail.com on 10 Apr 2008 at 8:55

GoogleCodeExporter commented 9 years ago 
Ah, you are right. Well, I can add a condition check but I am not sure if that 
is
necessary since we always use progress file. I changed it to the old way. Let me
know if you want me to change that.

New patch attached.

Original comment by jiayin...@gmail.com on 10 Apr 2008 at 9:18

Attachments:

GoogleCodeExporter commented 9 years ago 
Fixed with revision 1533.

Original comment by jiahotc...@gmail.com on 10 Apr 2008 at 10:32