dCache / oncrpc4j

Pure Java implementation of ONCRPC/SUNRPC
Other
30 stars 30 forks source link

OncRpcSvc constructor builds transports with hard-coded same-thread-io-strategy, rather than the one specified. #97

Closed jcalcote closed 1 year ago

jcalcote commented 1 year ago

I was monitoring my rpc service via jmx and noticed the io-strategy was set to same-thread. That seemed strange since I requested leader-follower in my code. I started examining oncrpc4j source code and noticed that the nio transport builders are hard-coding same-thread strategy rather than using the one configured by the oncrpcsrv builder:

        IoStrategy ioStrategy = builder.getIoStrategy();
        String serviceName = builder.getServiceName();
        ThreadPoolConfig selectorPoolConfig = getSelectorPoolCfg(ioStrategy,
                serviceName,
                builder.getSelectorThreadPoolSize());

        if ((protocol & IpProtocolType.TCP) != 0) {
            final TCPNIOTransport tcpTransport = TCPNIOTransportBuilder
                    .newInstance()
                    .setReuseAddress(true)
                    .setIOStrategy(SameThreadIOStrategy.getInstance())
                    .setSelectorThreadPoolConfig(selectorPoolConfig)
                    .setSelectorRunnersCount(selectorPoolConfig.getMaxPoolSize())
                    .build();
            _transports.add(tcpTransport);
        }

        if ((protocol & IpProtocolType.UDP) != 0) {
            final UDPNIOTransport udpTransport = UDPNIOTransportBuilder
                    .newInstance()
                    .setReuseAddress(true)
                    .setIOStrategy(SameThreadIOStrategy.getInstance())
                    .setSelectorThreadPoolConfig(selectorPoolConfig)
                    .setSelectorRunnersCount(selectorPoolConfig.getMaxPoolSize())
                    .build();
            _transports.add(udpTransport);
        }

Is this intentional?

It seems the specified io-strategy is only used to set the size of the thread pool.

jcalcote commented 1 year ago

I'm currently testing a custom build - if it works well, I'll submit a patch.

kofemann commented 1 year ago

Indeed, the strategy selection ignoring leader follow policy, probably as it was not considered as a one that will be used. So, PR is welcome.

kofemann commented 1 year ago

closed by 641d6cf 459920f 3813afc e50ceda