[sheepdog] [PATCH v4 1/2] lib: move sockfd cache from sheep to lib
Liu Yuan
namei.unix at gmail.com
Thu Jul 25 09:11:51 CEST 2013
On Thu, Jul 25, 2013 at 03:43:54PM +0900, Hitoshi Mitake wrote:
> On some subcommands, collie also issues lots of request to sheeps. So
> collie can enjoy sockfd caching. For this purpose, this patch moves
> sockfd from sheep to lib and generalize the interfaces of sockfd.
>
> This patch doesn't change anything related to IO NIC in sockfd
> cache. Because collie can know address and port of IO NIC so
> potentially use them.
>
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
> include/Makefile.am | 3 +-
> include/internal_proto.h | 2 +
> include/sockfd_cache.h | 23 ++
> lib/Makefile.am | 2 +-
> {sheep => lib}/sockfd_cache.c | 113 +++++----
> sheep/gateway.c | 10 +-
> sheep/group.c | 2 +-
> sheep/sheep.c | 2 +
> sheep/sheep_priv.h | 11 +-
> sheep/sockfd_cache.c | 509 +----------------------------------------
> 10 files changed, 93 insertions(+), 584 deletions(-)
> create mode 100644 include/sockfd_cache.h
> copy {sheep => lib}/sockfd_cache.c (90%)
>
> diff --git a/include/Makefile.am b/include/Makefile.am
> index 0acb76e..4d0c229 100644
> --- a/include/Makefile.am
> +++ b/include/Makefile.am
> @@ -2,4 +2,5 @@ MAINTAINERCLEANFILES = Makefile.in config.h.in
>
> noinst_HEADERS = bitops.h event.h logger.h sheepdog_proto.h util.h \
> list.h net.h sheep.h exits.h strbuf.h rbtree.h \
> - sha1.h option.h internal_proto.h shepherd.h work.h
> + sha1.h option.h internal_proto.h shepherd.h work.h \
> + sockfd_cache.h
> diff --git a/include/internal_proto.h b/include/internal_proto.h
> index 0463eae..0061007 100644
> --- a/include/internal_proto.h
> +++ b/include/internal_proto.h
> @@ -20,6 +20,8 @@
> #include <stdint.h>
> #include <netinet/in.h>
>
> +#include "sheepdog_proto.h"
> +
> #define SD_SHEEP_PROTO_VER 0x08
>
> #define SD_DEFAULT_COPIES 3
> diff --git a/include/sockfd_cache.h b/include/sockfd_cache.h
> new file mode 100644
> index 0000000..76154c9
> --- /dev/null
> +++ b/include/sockfd_cache.h
> @@ -0,0 +1,23 @@
> +#ifndef SOCKFD_CACHE_H
> +#define SOCKFD_CACHE_H
> +
> +#include "internal_proto.h"
> +#include "work.h"
> +
> +struct sockfd *sockfd_cache_get(const struct node_id *nid);
> +void do_sockfd_cache_put(const struct node_id *nid, int idx);
> +void sockfd_cache_put(const struct node_id *nid, struct sockfd *sfd);
> +void sockfd_node_del(const struct node_id *nid);
> +void sockfd_cache_del(const struct node_id *nid, struct sockfd *sfd);
> +void sockfd_cache_add(const struct node_id *nid);
> +void sockfd_cache_add_group(const struct sd_node *nodes, int nr);
> +
> +void init_sockfd(struct work_queue *wq);
> +
> +/* sockfd_cache */
> +struct sockfd {
> + int fd;
> + int idx;
> +};
> +
> +#endif /* SOCKFD_CACHE_H */
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index a95ce4c..22edf6e 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -5,7 +5,7 @@ INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
> noinst_LIBRARIES = libsheepdog.a
>
> libsheepdog_a_SOURCES = event.c logger.c net.c util.c rbtree.c strbuf.c \
> - sha1.c option.c work.c
> + sha1.c option.c work.c sockfd_cache.c
>
> # support for GNU Flymake
> check-syntax:
> diff --git a/sheep/sockfd_cache.c b/lib/sockfd_cache.c
> similarity index 90%
> copy from sheep/sockfd_cache.c
> copy to lib/sockfd_cache.c
> index 13bb7f6..334ba16 100644
> --- a/sheep/sockfd_cache.c
> +++ b/lib/sockfd_cache.c
> @@ -27,7 +27,13 @@
> * 7 support dual connections to a single node.
> */
>
> -#include "sheep_priv.h"
> +#include <pthread.h>
> +
> +#include "sockfd_cache.h"
> +#include "work.h"
> +#include "rbtree.h"
> +#include "util.h"
> +#include "sheep.h"
>
> struct sockfd_cache {
> struct rb_root root;
> @@ -208,20 +214,6 @@ false_out:
> return false;
> }
>
> -/* When node craches, we should delete it from the cache */
> -void sockfd_cache_del(const struct node_id *nid)
> -{
> - char name[INET6_ADDRSTRLEN];
> - int n;
> -
> - if (!sockfd_cache_destroy(nid))
> - return;
> -
> - n = uatomic_sub_return(&sockfd_cache.count, 1);
> - addr_to_str(name, sizeof(name), nid->addr, 0);
> - sd_dprintf("%s:%d, count %d", name, nid->port, n);
> -}
> -
> static void sockfd_cache_add_nolock(const struct node_id *nid)
> {
> struct sockfd_cache_entry *new = xmalloc(sizeof(*new));
> @@ -282,6 +274,8 @@ void sockfd_cache_add(const struct node_id *nid)
> static uatomic_bool fds_in_grow;
> static int fds_high_watermark = FDS_WATERMARK(DEFAULT_FDS_COUNT);
>
> +static struct work_queue *grow_wq;
> +
> static void do_grow_fds(struct work *work)
> {
> struct sockfd_cache_entry *entry;
> @@ -318,6 +312,15 @@ static inline void check_idx(int idx)
> {
> struct work *w;
>
> + if (!grow_wq) {
> + sd_dprintf("Request for growing fds is issued, but"
> + " a work queue for this work is not registered yet.");
> + sd_dprintf("Initializing work queue in earlier stage"
> + " is suggested.");
> +
> + return;
> + }
> +
> if (idx <= fds_high_watermark)
> return;
> if (!uatomic_set_true(&fds_in_grow))
> @@ -326,7 +329,7 @@ static inline void check_idx(int idx)
> w = xmalloc(sizeof(*w));
> w->fn = do_grow_fds;
> w->done = grow_fds_done;
> - queue_work(sys->sockfd_wqueue, w);
> + queue_work(grow_wq, w);
> }
>
> /* Add the node back if it is still alive */
> @@ -350,7 +353,7 @@ alive:
> }
>
> /* Try to create/get cached IO connection. If failed, fallback to non-IO one */
> -static struct sockfd *sockfd_cache_get(const struct node_id *nid)
> +static struct sockfd *do_sockfd_cache_get(const struct node_id *nid)
> {
> struct sockfd_cache_entry *entry;
> struct sockfd *sfd;
> @@ -402,7 +405,7 @@ out:
> return sfd;
> }
>
> -static void sockfd_cache_put(const struct node_id *nid, int idx)
> +void do_sockfd_cache_put(const struct node_id *nid, int idx)
> {
> bool use_io = nid->io_port ? true : false;
> const uint8_t *addr = use_io ? nid->io_addr : nid->addr;
> @@ -442,20 +445,29 @@ static void sockfd_cache_close(const struct node_id *nid, int idx)
> }
>
> /*
> + * set work queue for growing fds
> + * before this function called, growing cannot be done
> + */
> +void init_sockfd(struct work_queue *wq)
> +{
> + grow_wq = wq;
> +}
We use sockfd_ prefix as namespace. So use sockfd_init() instead. I think it is
better to push work queue init in sockfd_init() and make wq variable private to
sockfd cache.
You rely on the revalidate_node() to add the node, I think this assumption is
wrong though it works for now. Add a node explicitly please.
do_sockfd_cache_put()/do_sockfd_cache_get() are not good names.
sockfd_cache_{get, put}_short() is better.
You should brief in the commit what you did, such as what kind of funtions you
removed. You didn't clean up the .h files when you remove those functions.
Thanks
Yuan
More information about the sheepdog
mailing list