On Mon, Sep 3, 2012 at 6:20 PM, Liu Yuan <namei.u...@gmail.com> wrote:
> On 09/02/2012 08:59 PM, Yunkai Zhang wrote:
>> +static int forward_request_concurrently(struct sd_req *hdr,
>> + void *data, unsigned int *wlen,
>> + struct sd_vnode *target_vnodes[],
>> + struct sd_rsp target_rsps[],
>> + void *target_data[],
>> + int nr_targets)
>
> This interface looks unnecessarily complex to me. I'd suggest changes like
> following
> untested patch , it is more generic and simpler, simply adding one more
> caller provided
> buffer to read response data if any.
>
> Thanks,
> Yuan
>
> =====================================================
> diff --git a/sheep/gateway.c b/sheep/gateway.c
> index 41d712b..690f0ac 100644
> --- a/sheep/gateway.c
> +++ b/sheep/gateway.c
> @@ -150,10 +150,12 @@ static inline void pfd_info_init(struct write_info *wi,
> struct pfd_info *pi)
> *
> * Return error code if any one request fails.
> */
> -static int wait_forward_request(struct write_info *wi, struct sd_rsp *rsp)
> +static int wait_forward_request(struct write_info *wi, void *read_buffer)
> {
> int nr_sent, err_ret = SD_RES_SUCCESS, ret, pollret, i;
> - struct pfd_info pi;;
> + struct pfd_info pi;
> + struct sd_rsp rsp;
> + char *rb_pos = (char *)read_buffer;
> again:
> pfd_info_init(wi, &pi);
> pollret = poll(pi.pfds, pi.nr, -1);
> @@ -174,23 +176,37 @@ again:
> if (re & (POLLERR | POLLHUP | POLLNVAL)) {
> err_ret = SD_RES_NETWORK_ERROR;
> finish_one_write_err(wi, i);
> - } else if (re & POLLIN) {
> - if (do_read(pi.pfds[i].fd, rsp, sizeof(*rsp))) {
> + goto finish_write;
> + }
> + if (!(re & POLLIN)) {
> + eprintf("unhandled poll event\n");
> + goto finish_write;
> + }
> + if (do_read(pi.pfds[i].fd, &rsp, sizeof(rsp))) {
> + eprintf("remote node might have gone away\n");
> + err_ret = SD_RES_NETWORK_ERROR;
> + finish_one_write_err(wi, i);
> + goto finish_write;
> + }
> +
> + if (rsp.data_length && read_buffer) {
> + memcpy(rb_pos, wi->ent[i].nid,
> sizeof(wi->ent[i].nid));
> + rb_pos += sizeof(wi->ent[i].nid);
> + if (do_read(pi.pfds[i].fd, &read_buffer,
> + rsp.data_length)) {
> eprintf("remote node might have gone away\n");
> err_ret = SD_RES_NETWORK_ERROR;
> finish_one_write_err(wi, i);
> goto finish_write;
> }
> -
> - ret = rsp->result;
> - if (ret != SD_RES_SUCCESS) {
> - eprintf("fail %"PRIx32"\n", ret);
> - err_ret = ret;
> - }
> - finish_one_write(wi, i);
> - } else {
> - eprintf("unhandled poll event\n");
> + rb_pos += rsp.data_length;
> }
> + ret = rsp.result;
> + if (ret != SD_RES_SUCCESS) {
> + eprintf("fail %"PRIx32"\n", ret);
> + err_ret = ret;
> + }
> + finish_one_write(wi, i);
> }
> finish_write:
> if (wi->nr_sent > 0)
> @@ -225,11 +241,10 @@ static inline void gateway_init_fwd_hdr(struct sd_req
> *fwd, struct sd_req *hdr)
> fwd->proto_ver = SD_SHEEP_PROTO_VER;
> }
>
> -static int gateway_forward_request(struct request *req)
> +static int gateway_forward_request(struct request *req, void *read_buffer)
> {
> - int i, err_ret = SD_RES_SUCCESS, ret, local = -1;
> + int i, err_ret = SD_RES_SUCCESS, ret;
> unsigned wlen;
> - struct sd_rsp *rsp = (struct sd_rsp *)&req->rp;
> struct sd_vnode *v;
> struct sd_vnode *obj_vnodes[SD_MAX_COPIES];
> uint64_t oid = req->rq.obj.oid;
> @@ -253,10 +268,6 @@ static int gateway_forward_request(struct request *req)
> struct sockfd *sfd;
>
> v = obj_vnodes[i];
> - if (vnode_is_local(v)) {
> - local = i;
> - continue;
> - }
>
> sfd = sheep_get_sockfd(&v->nid);
> if (!sfd) {
> @@ -274,21 +285,9 @@ static int gateway_forward_request(struct request *req)
> write_info_advance(&wi, v, sfd);
> }
>
> - if (local != -1 && err_ret == SD_RES_SUCCESS) {
> - v = obj_vnodes[local];
> -
> - assert(op);
> - ret = sheep_do_op_work(op, req);
> -
> - if (ret != SD_RES_SUCCESS) {
> - eprintf("fail to write local %"PRIx32"\n", ret);
> - err_ret = ret;
> - }
> - }
> -
> dprintf("nr_sent %d, err %x\n", wi.nr_sent, err_ret);
> if (wi.nr_sent > 0) {
> - ret = wait_forward_request(&wi, rsp);
> + ret = wait_forward_request(&wi, read_buffer);
> if (ret != SD_RES_SUCCESS)
> err_ret = ret;
> }
> @@ -301,7 +300,7 @@ int gateway_write_obj(struct request *req)
> if (sys->enable_write_cache && !req->local &&
> !bypass_object_cache(req))
> return object_cache_handle_request(req);
>
> - return gateway_forward_request(req);
> + return gateway_forward_request(req, NULL);
> }
>
> int gateway_create_and_write_obj(struct request *req)
> @@ -309,10 +308,10 @@ int gateway_create_and_write_obj(struct request *req)
> if (sys->enable_write_cache && !req->local &&
> !bypass_object_cache(req))
> return object_cache_handle_request(req);
>
> - return gateway_forward_request(req);
> + return gateway_forward_request(req, NULL);
> }
>
> int gateway_remove_obj(struct request *req)
> {
> - return gateway_forward_request(req);
> + return gateway_forward_request(req, NULL);
> }
It's ok for me.
--
Yunkai Zhang
Work at Taobao
--
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog
