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

[BUG] RedisClient created by PeriodicTaskManager doesn't close connection #975

Open
HuangShaoyan opened this issue Nov 27, 2024 · 4 comments · May be fixed by #977
Open

[BUG] RedisClient created by PeriodicTaskManager doesn't close connection #975

HuangShaoyan opened this issue Nov 27, 2024 · 4 comments · May be fixed by #977
Assignees
Labels
bug Something isn't working

Comments

@HuangShaoyan
Copy link

Describe the bug
asynq creates a scheduler using NewScheduler when invoking NewPeriodicTaskManager, setting scheduler.sharedConnection to false. However, scheduler.client.sharedConnection remains true.
It will cause Client.Close to print redis connection is shared so the Client can't be closed through asynq.

Environment:

  • OS: MacOS
  • asynq v0.25.0
  • Redis 7.0

To Reproduce

  1. Create a PeriodicTaskManager using asynq.NewPeriodicTaskManager.
  2. Press Ctrl+C to stop the process.
  3. Check the log output.

Expected behavior
asynq should set scheduler.client.sharedConnection to false when the Redis client is created by the PeriodicTaskManager itself.

@HuangShaoyan HuangShaoyan added the bug Something isn't working label Nov 27, 2024
@kamikazechaser
Copy link
Collaborator

kamikazechaser commented Dec 3, 2024

Probably landed in #958. Pinging @daixijun as well.

Actually landed in #742. I'll review the entire PR again.

@kamikazechaser
Copy link
Collaborator

I can reproduce it:

asynq: pid=20593 2024/12/03 05:57:22.303908 INFO: Scheduler starting
asynq: pid=20593 2024/12/03 05:57:22.303939 INFO: Scheduler timezone is set to UTC
asynq: pid=20593 2024/12/03 05:57:22.303945 INFO: Send signal TERM or INT to stop the scheduler
^Casynq: pid=20593 2024/12/03 05:57:22.781738 INFO: Scheduler shutting down
asynq: pid=20593 2024/12/03 05:57:22.783083 ERROR: Failed to close redis client connection: redis connection is shared so the Client can't be closed through asynq
asynq: pid=20593 2024/12/03 05:57:22.783122 INFO: Scheduler stopped
2024/12/03 08:57:42 graceful shutdown period exceeded, forcefully shutting down
exit status 1

@kamikazechaser
Copy link
Collaborator

kamikazechaser commented Dec 3, 2024

@HuangShaoyan I have pushed a fix pending review. Could you try it out? https://github.com/hibiken/asynq/tree/sohail/pm-redis-conn-hotfix

go get github.com/hibiken/asynq@f1e7dc4056abaa6549bf014d2e6b67508437d236

@HuangShaoyan
Copy link
Author

I have tested the fix f1e7dc, and I can confirm that the bug has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants