Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javadoc mentions Delay.exponential() is capped at 30 milliseconds #799

Closed
phxql opened this issue Jun 13, 2018 · 4 comments
Closed

Javadoc mentions Delay.exponential() is capped at 30 milliseconds #799

phxql opened this issue Jun 13, 2018 · 4 comments
Labels
type: bug A general bug
Milestone

Comments

@phxql
Copy link

phxql commented Jun 13, 2018

Hi,

the documentation says in https://github.com/lettuce-io/lettuce-core/wiki/Configuring-Client-resources on reconnect delay that the upper limit is by default 30 seconds. If i take a look at the implementation in Lettuce 4.4.4, (com.lambdaworks.redis.resource.Delay#exponential(), which is used by default), the upperbound is 30 milliseconds, not seconds.

@mp911de mp911de added the type: bug A general bug label Jun 13, 2018
@mp911de
Copy link
Collaborator

mp911de commented Jun 13, 2018

Thanks for filing an issue. I think there are two things here:

  1. The Javadoc says the upper limit is 30 milliseconds as per https://github.com/lettuce-io/lettuce-core/blob/42c51e60201c5da3e3f1be379c2f81e29de86cae/src/main/java/com/lambdaworks/redis/resource/Delay.java#L113
  2. The actual upper bound is 30 second (30000 milliseconds).
        Delay exponential = Delay.exponential();
        for (int i = 0; i < 32; i++) {
            System.out.println(exponential.createDelay(i));
        }

results in an output of:


0
1
2
4
8
16
32
64
128
256
512
1024
2048
4096
8192
16384
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000
30000

Out of curiosity, how did you come across this issue?

@phxql
Copy link
Author

phxql commented Jun 13, 2018

Ah yeah, you're right. Only the milliseconds in the Javadocs are wrong.

I tried to find the cause for a problem where the Logger for com.lambdaworks.redis.protocol.ConnectionWatchdog and com.lambdaworks.redis.RedisClient where spamming our logs on WARN every millisecond when our redis sentinel went down. I assumed that would be due to the reconnection delay not doing exponential backoff and start digging in the code.

But if the exponential delay is indeed working correct, then the cause must be something else.

This is an example of that:

{"@timestamp":1528886913021,"level":"WARN","thread":"lettuce-eventExecutorLoop-1-4","logger":"com.lambdaworks.redis.RedisClient","message":"Cannot connect Redis Sentinel at RedisURI [host='sentinel-cluster', port=26379]: com.lambdaworks. 
{"@timestamp":1528886913026,"level":"WARN","thread":"lettuce-eventExecutorLoop-6-1","logger":"com.lambdaworks.redis.RedisClient","message":"Cannot connect Redis Sentinel at RedisURI [host='sentinel-cluster', port=26379]: com.lambdaworks. 
{"@timestamp":1528886913027,"level":"WARN","thread":"lettuce-eventExecutorLoop-1-4","logger":"com.lambdaworks.redis.RedisClient","message":"Cannot connect Redis Sentinel at RedisURI [host='sentinel-cluster', port=26379]: com.lambdaworks. 
{"@timestamp":1528886913028,"level":"WARN","thread":"lettuce-eventExecutorLoop-6-4","logger":"com.lambdaworks.redis.RedisClient","message":"Cannot connect Redis Sentinel at RedisURI [host='sentinel-cluster', port=26379]: com.lambdaworks. 
{"@timestamp":1528886913030,"level":"WARN","thread":"lettuce-eventExecutorLoop-1-2","logger":"com.lambdaworks.redis.RedisClient","message":"Cannot connect Redis Sentinel at RedisURI [host='sentinel-cluster', port=26379]: com.lambdaworks. 
{"@timestamp":1528886913031,"level":"WARN","thread":"lettuce-eventExecutorLoop-6-1","logger":"com.lambdaworks.redis.RedisClient","message":"Cannot connect Redis Sentinel at RedisURI [host='sentinel-cluster', port=26379

@mp911de
Copy link
Collaborator

mp911de commented Jun 14, 2018

The log message you're seeing originates from calling RedisClient.connectSentinel(…). Looking at the thread, I find it unfortunate to use the Lettuce worker pool for this kind of operation as the threads might interfere with each other. This is because connectSentinel(…) is a blocking method and this does not play well when using other threads than the main application thread.

@mp911de mp911de changed the title Default reconnection delay 30 seconds or milliseconds? Javadoc mentions Delay.exponential() is capped at 30 milliseconds Jun 14, 2018
mp911de added a commit that referenced this issue Jun 14, 2018
mp911de added a commit that referenced this issue Jun 14, 2018
mp911de added a commit that referenced this issue Jun 14, 2018
mp911de added a commit that referenced this issue Jun 14, 2018
@mp911de
Copy link
Collaborator

mp911de commented Jun 14, 2018

That's fixed now. Closing this ticket.

Feel free to file a new issue if you run into problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants