[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