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

Prevents the program from entering endless loop #144

Conversation

hbina
Copy link
Contributor

@hbina hbina commented Oct 18, 2020

See #135

The fix is simple enough, we just abort if we detect this particular
setting.

Signed-off-by: Hanif Bin Ariffin hanif.ariffin.4326@gmail.com

@yossigo
Copy link
Contributor

yossigo commented Oct 22, 2020

Hi @hbina, thanks for this PR. Did you consider fixing the root cause of the problem?

@hbina
Copy link
Contributor Author

hbina commented Oct 22, 2020

From what I understand, this seems like am expected behavior. If I am requesting 5 request and also say I will reset at every 5 request, I will just do this endlessly. Maybe I am wrong.

@yossigo
Copy link
Contributor

yossigo commented Oct 22, 2020

@hbina reconnect-interval specifies a maximum number of commands to execute on a connection before dropping it and re-establishing a new one. I believe it's just a bug in the implementation, but you are more than welcome to take a look for yourself. Thanks!

@hbina
Copy link
Contributor Author

hbina commented Oct 23, 2020

Okay, I will take a look.

The application will no longer abort but instead perform the benchmark
without entering the endless loop.

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@hbina hbina force-pushed the prevent_endless_loop_reconnect_interval_requests branch from 193523d to 722ccac Compare October 23, 2020 07:20
@hbina
Copy link
Contributor Author

hbina commented Oct 23, 2020

Is this correct? I hope I am not messing with other stuff. This does prevent the endless loop

@yossigo

@yossigo
Copy link
Contributor

yossigo commented Oct 26, 2020

@hbina Thank you, looks good!

@yossigo yossigo merged commit 28647e8 into RedisLabs:master Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants