[tarantool-patches] Re: [PATCH v2] iproto: introduce a proxy module.

Georgy Kirichenko georgy at tarantool.org
Thu Oct 4 14:54:27 MSK 2018


Hi and thank you for the patch! Overall it looks good but I have some 
comments.
 - I would like if you created a dedicated fiber pool and cbus endpoint to
   perform IPROTO<->TX interconnection. In that case iproto and tx
   initialization could be more clearly splitted.
 - I think we could have only one hash table to locate a corresponding
   connection with "username at uri" instead of two-level routing.
 - It should be fine to introduce some error injections.
 - If you allocate and free a lots of same space objects then mempool
   is better option than (m/c)alloc/free

Also I have some comments inline.
> +
> +	instance->is_ro = is_ro;
> +	instance->is_local = is_local;
> +	if (is_local) {
> +		/* There can be only one local instance. */
> +		assert(local_instance == NULL);
> +		assert(box_is_configured());
This function works in the IPROTO thread and calling TX threaded function may 
lead to undefined behavior.
> +		local_instance = instance;
> +	} else if (uri_parse(&instance->uri, uri) || !instance->uri.service) {
> +			free(instance);
> +			diag_set(ClientError, ER_CFG, "uri",
> +				 "expected host:service or /unix.socket");
> +			return NULL;
> +	}
> +
> +	rlist_add_tail(&proxy_instances, &instance->link);
> +

> +
> +		if (!is_proxy_configured) {
> +			/*
> +			 * This can't throw, but should not be
> +			 * done in case of exception.
> +			 */
> +process_locally:
> +			cpipe_push_input(&tx_pipe, &msg->base);
> +			continue;
> +		}
> +
> +		struct proxy_instance *instance = proxy_find_instance(msg);
> +		if (instance == NULL) {
I think some actions with request should be done, at least clearing an input 
buffer space.
> +			diag_log();
> +			continue;
> +		}
> +		if (instance == local_instance) {
> +			if (msg->header.type != IPROTO_AUTH)
> +				goto process_locally;
> +			struct proxy_auth_msg *m = (struct proxy_auth_msg 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20181004/1b5f0105/attachment.sig>


More information about the Tarantool-patches mailing list