On Fri, Jul 16, 2021 at 10:36 AM Ken Giusti <[email protected]> wrote:
> On Fri, Jul 16, 2021 at 9:39 AM Ted Ross <[email protected]> wrote: > > > Hi Jiri, > > > > In normal operation of the router, there is only one call to 'qd_router', > > so I'm not sure we really need 'type_registered'. That code has been > there > > from the very beginning so I don't think it was added to fix a particular > > problem. I think it could safely be removed. > > > > > Hey Ted - do you think there'd be a benefit in terms of reduced code if we > refactor out the old generic node-based dispatcher layer? > Probably. There's been talk in the past about eliminating the "container" layer altogether thus removing one level of callbacks. I think the right approach would be to refactor the Proton interface to be a protocol adaptor. This would have the added benefit of allowing management hooks for AMQP access via Skupper. But, like you said, probably not a high priority. > > Not a high priority of course, but if there's ways to simplify the codebase > that would be great. > > -K > > > > > -Ted > > > > On Fri, Jul 16, 2021 at 5:56 AM Jiri Daněk <[email protected]> wrote: > > > > > Hello folks > > > > > > ``` > > > static int type_registered = 0; > > > > > > qd_router_t *qd_router(qd_dispatch_t *qd, qd_router_mode_t mode, const > > char > > > *area, const char *id) > > > { > > > if (!type_registered) { > > > type_registered = 1; > > > qd_container_register_node_type(qd, &router_node); > > > } > > > ``` > > > > > > > > > https://github.com/apache/qpid-dispatch/blob/d8800269d3a80225794be9914b5fc9f8d6118d04/src/router_node.c#L1623-L1630 > > > > > > I'd like to understand the motivation behind this code better. > > > > > > Some parts of the codebase assume that there may be many qd_dispatch_t > > > instances around, while others assume there is only one. There is the > > > `dispatch` global variable in python_embedded.c, there is the global > flag > > > `type_registered` here, but the `qd_dispatch_t` pointer is usually > passed > > > through function argument (as if there could be more than one). > > > > > > Having this check for `type_registered` prevents me from stopping and > > > freeing one instance of a router, then immediately starting another in > > the > > > same process. I want to do this for testing purposes. What happens now > is > > > that the second router I start will not function correctly; deleting > this > > > type_registered logic makes it work right (as far as my tests so far > are > > > concerned). > > > > > > It seems to me that it should be perfectly ok to have multiple dispatch > > > instances in the single process, as long as there is only one at a > time. > > > -- > > > Mit freundlichen Grüßen / Kind regards > > > Jiri Daněk > > > > > > > > -- > -K >
