mp911de / spinach

Scalable Java Disque client
Apache License 2.0
34 stars 4 forks source link

Command time out not using the provided value. #28

Closed odiel closed 8 years ago

odiel commented 8 years ago

Hi I'm trying to have a process listening forever for new jobs coming into a queue, for that I'm doing the following code:

import biz.paluch.spinach.DisqueClient;
import biz.paluch.spinach.DisqueURI;
import biz.paluch.spinach.api.DisqueConnection;
import biz.paluch.spinach.api.Job;
import biz.paluch.spinach.api.sync.DisqueCommands;

import java.util.concurrent.TimeUnit;

public class Test {

    public static void main(String args[]) throws Exception {
        DisqueClient client = DisqueClient.create(DisqueURI.create("192.168.133.130", 7711));
        DisqueConnection<String, String> connection = client.connect();
        DisqueCommands<String, String> sync = connection.sync();

        Job<String, String> job;

        while (true) {
            job = sync.getjob(24, TimeUnit.HOURS, "my_queue");
        }
    }
}

But the process times out a minute after the queue is empty. Tracking down how this happened I ended up here:

File: \biz\paluch\redis\spinach\0.3\spinach-0.3-sources.jar!\biz\paluch\spinach\impl\FutureSyncInvocationHandler.java

   @Override
    @SuppressWarnings("unchecked")
    protected Object handleInvocation(Object proxy, Method method, Object[] args) throws Throwable {

        try {

            Method targetMethod = asyncApi.getClass().getMethod(method.getName(), method.getParameterTypes());

            Object result = targetMethod.invoke(asyncApi, args);

            if (result instanceof RedisCommand) {
                RedisCommand<?, ?, ?> command = (RedisCommand<?, ?, ?>) result;

                return LettuceFutures.await(command, connection.getTimeout(), connection.getTimeoutUnit());
            }

            return result;

        } catch (InvocationTargetException e) {
            throw e.getTargetException();
        }

    }

I think the method should be using the provided timeout instead of the connection one.

mp911de commented 8 years ago

Hi, you're using the synchronous API which is guarded by the connection timeout settings. This timeout setting applies to all synchronous method calls and does not depend on method arguments. You can set the timeout on DisqueConnection using setTimeout because there's another bug where the timeout from DisqueURI is not applied to the connection timeout settings (#29).

mp911de commented 8 years ago

Closing, no further activity.