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

Fix Issue113 #114

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Fix Issue113 #114

merged 1 commit into from
Sep 11, 2015

Conversation

sunheehnus
Copy link
Contributor

No description provided.

@sunheehnus
Copy link
Contributor Author

Dummy ack job did not got removed when clusterSize is 1,

When clusterSize is 1, tryJobGC will not GC the dummy job and gotAckReceived will not be triggered either.


TEST CASE (a one-node cluster)

Before

127.0.0.1:7711> info jobs
# Jobs
registered_jobs:0
127.0.0.1:7711> ackjob DI77455215a7aabc748fdb448dd59bc3e891abf16905a0SQ
(integer) 0
127.0.0.1:7711> info jobs
# Jobs
registered_jobs:1

After

127.0.0.1:7711> info jobs
# Jobs
registered_jobs:0
127.0.0.1:7711> ackjob DI77455215a7aabc748fdb448dd59bc3e891abf16905a0SQ
(integer) 0
127.0.0.1:7711> info jobs
# Jobs
registered_jobs:0

antirez added a commit that referenced this pull request Sep 11, 2015
@antirez antirez merged commit eac57d6 into antirez:master Sep 11, 2015
@antirez
Copy link
Owner

antirez commented Sep 11, 2015

Thanks @sunheehnus! I merged your commit, but I'm going to change a few things and comment here what I changed so that you may review. Thank you a lot as usually!

@antirez
Copy link
Owner

antirez commented Sep 11, 2015

Just reworked a bit the fix in three commits:

d947415 Don't create a dummy ack if the cluster size is 1.
c8cc037 jscanCallback(): check for dummy job in a consistent way.
045562b Two different "all nodes reached" code paths unified in tryJobGC().

Thanks!

@sunheehnus
Copy link
Contributor Author

All look good to me. 👍
Sorry for the delay, on a trip last 3 days without stable access to Internet. :-)

@sunheehnus sunheehnus deleted the issue113 branch September 25, 2015 13:52
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