[sheepdog] [PATCH v1 1/4] sheep/http: add option 'buffer=' to adjust size of buffer for kv

[Liu Yuan]

On Wed, Jan 29, 2014 at 05:15:04PM +0800, Robin Dong wrote:
> In some testing environment, large size of buffer will make
> the performance of swift bad because kv will split buffer into too many
> requests and send out, which lead to congestion in network; but small size
> of buffer will make the speed of receiving http request slow. After all,
> we found out that 8MB is the best value for our environment.
> 
> Even the 8MB is good, the size of buffer is difficult to choose in different
> environment (different hardware of servers, different number of nodes, etc.),
> so we should add this option to let users choose buffer size.

> Signed-off-by: Robin Dong <sanbai at taobao.com>
> ---
>  sheep/http/http.c | 16 ++++++++++++++++
>  sheep/http/http.h |  4 ++++
>  sheep/http/kv.c   |  8 ++++----
>  sheep/sheep.c     |  5 +++--
>  4 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/sheep/http/http.c b/sheep/http/http.c
> index cf658c6..c0b3287 100644
> --- a/sheep/http/http.c
> +++ b/sheep/http/http.c
> @@ -346,6 +346,21 @@ static int http_opt_port_parser(const char *s)
>  	return 0;
>  }
>  
> +#define UNIT		(UINT64_C(1) << 20)
> +#define MIN_RW_BUFFER	(SD_DATA_OBJ_SIZE / UNIT)
> +#define MAX_RW_BUFFER	1024
> +static int http_opt_buffer_parser(const char *s)
> +{
> +	char *endp;
> +	uint64_t val;
> +
> +	val = strtoll(s, &endp, 10);
> +	if (s != endp && val > MIN_RW_BUFFER && val < MAX_RW_BUFFER)
> +		kv_rw_buffer = round_up(val, MIN_RW_BUFFER);
> +	sd_debug("kv_rw_buffer: %"PRIu64"MB", kv_rw_buffer);
> +	return 0;
> +}
> +
>  static int http_opt_default_parser(const char *s)
>  {
>  	struct http_driver *hdrv;
> @@ -375,6 +390,7 @@ static int http_opt_default_parser(const char *s)
>  static struct option_parser http_opt_parsers[] = {
>  	{ "host=", http_opt_host_parser },
>  	{ "port=", http_opt_port_parser },
> +	{ "buffer=", http_opt_buffer_parser },
>  	{ "", http_opt_default_parser },
>  	{ NULL, NULL },
>  };
> diff --git a/sheep/http/http.h b/sheep/http/http.h
> index f437f82..21006b2 100644
> --- a/sheep/http/http.h
> +++ b/sheep/http/http.h
> @@ -115,6 +115,10 @@ int http_request_writef(struct http_request *req, const char *fmt, ...);
>  #define SD_MAX_BUCKET_NAME 256
>  #define SD_MAX_OBJECT_NAME 1024
>  
> +/* This default value shows best performance in test */
> +#define DEFAULT_KV_RW_BUFFER 8 

I suggest make it as 32M for defualt your help message says

>
> +extern uint64_t kv_rw_buffer;
> +
>  /* Account operations */
>  int kv_create_account(const char *account);
>  int kv_read_account_meta(struct http_request *req, const char *account);
> diff --git a/sheep/http/kv.c b/sheep/http/kv.c
> index bfbf305..f896ab1 100644
> --- a/sheep/http/kv.c
> +++ b/sheep/http/kv.c
> @@ -16,6 +16,8 @@
>  #include "sheep_priv.h"
>  #include "http.h"
>  
> +uint64_t kv_rw_buffer = DEFAULT_KV_RW_BUFFER;
> +
>  struct kv_bnode {
>  	char name[SD_MAX_BUCKET_NAME];
>  	uint64_t object_count;
> @@ -673,8 +675,6 @@ static int vdi_read_write(uint32_t vid, char *data, size_t length,
>  	return local_req_wait(iocb);
>  }
>  
> -#define MAX_RW_BUFFER (SD_DATA_OBJ_SIZE * 25) /* No rationale yet */
> -
>  static int onode_populate_extents(struct kv_onode *onode,
>  				  struct http_request *req)
>  {
> @@ -683,7 +683,7 @@ static int onode_populate_extents(struct kv_onode *onode,
>  	int ret;
>  	char *data_buf = NULL;
>  	uint32_t data_vid = onode->data_vid;
> -	uint64_t write_buffer_size = MIN(MAX_RW_BUFFER, req->data_length);
> +	uint64_t write_buffer_size = MIN(kv_rw_buffer, req->data_length);
>  
>  	count = DIV_ROUND_UP(req->data_length, SD_DATA_OBJ_SIZE);
>  	sys->cdrv->lock(data_vid);
> @@ -865,7 +865,7 @@ static int onode_read_extents(struct kv_onode *onode, struct http_request *req)
>  	uint64_t off = req->offset, len = req->data_length;
>  	int ret;
>  	char *data_buf = NULL;
> -	uint64_t read_buffer_size = MIN(MAX_RW_BUFFER, onode->size);
> +	uint64_t read_buffer_size = MIN(kv_rw_buffer, onode->size);
>  
>  	data_buf = xmalloc(read_buffer_size);
>  	total_size = len;
> diff --git a/sheep/sheep.c b/sheep/sheep.c
> index 7ef3746..7c1eaec 100644
> --- a/sheep/sheep.c
> +++ b/sheep/sheep.c
> @@ -47,10 +47,11 @@ static const char http_help[] =
>  "Available arguments:\n"
>  "\thost=: specify a host to communicate with http server (default: localhost)\n"
>  "\tport=: specify a port to communicate with http server (default: 8000)\n"
> +"\tbuffer=: specify buffer size (unit: MB) for http request (default: 32)\n"
>  "\tswift: enable swift API\n"
> -"Example:\n\t$ sheep -r host=localhost,port=7001,swift ...\n"
> +"Example:\n\t$ sheep -r host=localhost,port=7001,buffer=64,swift ...\n"
>  "This tries to enable Swift API and use localhost:7001 to\n"
> -"communicate with http server.\n";
> +"communicate with http server, using 64MB buffer.\n";

we should support 'buffer=64M' notation instead of 'buffer=64'
  
Thanks
Yuan