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

Eliminate use of transaction so it can be used w/sentinel (via predis) #194

Closed
wants to merge 3 commits into from
Closed

Conversation

pdbreen
Copy link
Contributor

@pdbreen pdbreen commented Oct 11, 2017

For #193. Eliminates use of transaction for straight read/delete. High availability is far more important to me than ensuring accuracy of stats, which I think would be the only possible side effect of the removal.

@pdbreen
Copy link
Contributor Author

pdbreen commented Oct 11, 2017

While no longer failing w/exception, metric collection is still not working properly w/sentinel. Within metrics dashboard, job names are missing initial characters and queue names show "false". Seems smembers within measuredJobs/measuredQueues is not including the prefix that substr is stripping....

Edit: covered/repaired in #196. Happened to be triggered by not establishing default prefix in sentinel config of horizon connection.

@taylorotwell
Copy link
Member

There is other things here besides the transaction. Please only do one thing per PR.

@pdbreen
Copy link
Contributor Author

pdbreen commented Oct 11, 2017

Sorry - wasn't aware this PR got modified when I promoted fix for #196 on my fork (I need both changes locally). Will resubmit this change in isolation.

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