)]}'
{"taskflow/jobs/backends/impl_redis.py":[{"author":{"_account_id":10584,"name":"Min Pae","username":"sputnik13","inactive":true},"change_message_id":"dcebed38b65a8beec0cc3452d414ed73e268d52c","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            return ru.get_expiry(self._client, owner_key,"},{"line_number":96,"context_line":"                                 prior_version\u003dself._redis_version)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def extend(self, expiry):"},{"line_number":99,"context_line":"        \"\"\"Extends the expiry for this job."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        Returns True if the expiry request was performed otherwise False."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_2961e039","line":98,"updated":"2015-05-02 04:29:57.000000000","message":"extend_expiration or extend_expiry","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"9550383ca8802658530d3388dfe22db7386bf01c","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            return ru.get_expiry(self._client, owner_key,"},{"line_number":96,"context_line":"                                 prior_version\u003dself._redis_version)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def extend(self, expiry):"},{"line_number":99,"context_line":"        \"\"\"Extends the expiry for this job."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        Returns True if the expiry request was performed otherwise False."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_a9dcd0e4","line":98,"in_reply_to":"3acd31a7_2961e039","updated":"2015-05-02 04:36:21.000000000","message":"Sounds good, wasn\u0027t sure what to name this :)","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":10584,"name":"Min Pae","username":"sputnik13","inactive":true},"change_message_id":"dcebed38b65a8beec0cc3452d414ed73e268d52c","unresolved":false,"context_lines":[{"line_number":285,"context_line":"local ms_expiry \u003d tonumber(ARGV[3])"},{"line_number":286,"context_line":"local result \u003d {}"},{"line_number":287,"context_line":"if redis.call(\"hexists\", listings_key, job_key) \u003d\u003d 1 then"},{"line_number":288,"context_line":"    if redis.call(\"exists\", owner_key) \u003d\u003d 1 then"},{"line_number":289,"context_line":"        local owner \u003d redis.call(\"get\", owner_key)"},{"line_number":290,"context_line":"        if owner \u003d\u003d expected_owner then"},{"line_number":291,"context_line":"            -- Owner is the same, leave it alone..."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_c986ecda","line":288,"updated":"2015-05-02 04:29:57.000000000","message":"Creating a key/value pair to lock a job seems susceptible to race conditions where two workers come to claim a job, both check for existence of owner_key, both get a negative response, then they both set, which results in both believing they own the job but only one really does.  Redis has a mechanism for check and set like behavior (http://redis.io/topics/transactions#cas) that seems like it would be better to use.","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"aeb458ed1d4540c6eb9d63354fab716a07ca2c37","unresolved":false,"context_lines":[{"line_number":285,"context_line":"local ms_expiry \u003d tonumber(ARGV[3])"},{"line_number":286,"context_line":"local result \u003d {}"},{"line_number":287,"context_line":"if redis.call(\"hexists\", listings_key, job_key) \u003d\u003d 1 then"},{"line_number":288,"context_line":"    if redis.call(\"exists\", owner_key) \u003d\u003d 1 then"},{"line_number":289,"context_line":"        local owner \u003d redis.call(\"get\", owner_key)"},{"line_number":290,"context_line":"        if owner \u003d\u003d expected_owner then"},{"line_number":291,"context_line":"            -- Owner is the same, leave it alone..."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_699478e1","line":288,"in_reply_to":"3acd31a7_69fd9839","updated":"2015-05-02 05:12:05.000000000","message":"Another thought, we might be able to reorder this to not have any if statements, and sacrifice the error reporting that they give, and order the operations so that under transaction failure (and due to lack of rollback) that it still works as expected; but I\u0027m meh on that sort of, even if this lua stuff is a little odd, it does seem to have its uses to do these operations in bulk, in an atomic way (and allows us to learn of errors that occur during that, even if its a little PITA to figure out).","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":10584,"name":"Min Pae","username":"sputnik13","inactive":true},"change_message_id":"00acb8c301c13e0531881567114588f8d8b4091e","unresolved":false,"context_lines":[{"line_number":285,"context_line":"local ms_expiry \u003d tonumber(ARGV[3])"},{"line_number":286,"context_line":"local result \u003d {}"},{"line_number":287,"context_line":"if redis.call(\"hexists\", listings_key, job_key) \u003d\u003d 1 then"},{"line_number":288,"context_line":"    if redis.call(\"exists\", owner_key) \u003d\u003d 1 then"},{"line_number":289,"context_line":"        local owner \u003d redis.call(\"get\", owner_key)"},{"line_number":290,"context_line":"        if owner \u003d\u003d expected_owner then"},{"line_number":291,"context_line":"            -- Owner is the same, leave it alone..."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_299680f0","line":288,"in_reply_to":"3acd31a7_69fd9839","updated":"2015-05-02 04:57:55.000000000","message":"I stand corrected :)","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"60f01855d52e9612ce5f95e06064ba719330d0b1","unresolved":false,"context_lines":[{"line_number":285,"context_line":"local ms_expiry \u003d tonumber(ARGV[3])"},{"line_number":286,"context_line":"local result \u003d {}"},{"line_number":287,"context_line":"if redis.call(\"hexists\", listings_key, job_key) \u003d\u003d 1 then"},{"line_number":288,"context_line":"    if redis.call(\"exists\", owner_key) \u003d\u003d 1 then"},{"line_number":289,"context_line":"        local owner \u003d redis.call(\"get\", owner_key)"},{"line_number":290,"context_line":"        if owner \u003d\u003d expected_owner then"},{"line_number":291,"context_line":"            -- Owner is the same, leave it alone..."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_c9eb8c77","line":288,"in_reply_to":"3acd31a7_c986ecda","updated":"2015-05-02 04:49:47.000000000","message":"The lua script is ran serially, so when this runs no other code will be mutating the backend.\n\nhttp://redis.io/commands/eval#atomicity-of-scripts\n\nThe reason I did lua here, instead of transactions (which I tried with also) was the thinking that this to correctly do this in a transaction we would need to set a \u0027big\u0027 watch on the listing key (which has all the jobs in a hash) to ensure that the job (or other keys) doesn\u0027t go bye-bye will we are doing the transaction. That seems like a big lock to take to avoid that situation (when a lua script can avoid that entirely via the guaranteed atomicity).\n\nThe other reason lua was picked is that we need to set multiple keys in one go (and if say the last key setting fails due to the transaction/watch detecting that a key has been modified) then since redis doesn\u0027t do rollback(s) that will basically result in the redis backend being in some sort of partially consistent state. Using lua scripts and doing those bulk sets in it can not be done in parallel so there is no such contention that could happen in the first place.\n\nhttp://redis.io/topics/transactions#why-redis-does-not-support-roll-backs\n\nAnother reason is that we have interdependent paths here (if statements that check the owner and bail out...), and transactions can\u0027t do that :-/\n\nhttp://www.redisgreen.net/blog/intro-to-lua-for-redis-programmers/#when-to-use-lua (some good examples there)\n\nSo overall after thinking, it seems like having some little lua scripts wasn\u0027t that bad (even if its somewhat convoluted); I don\u0027t expect them to be a bottleneck (they aren\u0027t complicated) but they should hopefully ensure that the backend is more consistent (vs with transactions where it may not be that consistent).","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"c9ca9b3204d2f1b7e3d6bd3ed6882e44b91e7af7","unresolved":false,"context_lines":[{"line_number":285,"context_line":"local ms_expiry \u003d tonumber(ARGV[3])"},{"line_number":286,"context_line":"local result \u003d {}"},{"line_number":287,"context_line":"if redis.call(\"hexists\", listings_key, job_key) \u003d\u003d 1 then"},{"line_number":288,"context_line":"    if redis.call(\"exists\", owner_key) \u003d\u003d 1 then"},{"line_number":289,"context_line":"        local owner \u003d redis.call(\"get\", owner_key)"},{"line_number":290,"context_line":"        if owner \u003d\u003d expected_owner then"},{"line_number":291,"context_line":"            -- Owner is the same, leave it alone..."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_69fd9839","line":288,"in_reply_to":"3acd31a7_c9eb8c77","updated":"2015-05-02 04:51:21.000000000","message":"Another good one:\n\nhttps://redislabs.com/blog/5-key-takeaways-for-developing-with-redis#tip5\n\nI got quite a few that I found (decent amount of tutorials online).","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":10584,"name":"Min Pae","username":"sputnik13","inactive":true},"change_message_id":"dcebed38b65a8beec0cc3452d414ed73e268d52c","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        \"\"\"Create and return a namespaced key from many segments."},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"        NOTE(harlowja): all pieces that are text/unicode are converted into"},{"line_number":426,"context_line":"        there binary equivalent (if they are already binary no conversion"},{"line_number":427,"context_line":"        takes place) before being joined (as redis expects binary keys and not"},{"line_number":428,"context_line":"        unicode/text ones)."},{"line_number":429,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_0964e446","line":426,"updated":"2015-05-02 04:29:57.000000000","message":"there -\u003e their","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"}],"taskflow/tests/unit/jobs/test_redis_job.py":[{"author":{"_account_id":10584,"name":"Min Pae","username":"sputnik13","inactive":true},"change_message_id":"dcebed38b65a8beec0cc3452d414ed73e268d52c","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    impl_redis.RedisJobBoard.MIN_REDIS_VERSION)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"@testtools.skipIf(not REDIS_AVAILABLE, \u0027redis is not available\u0027)"},{"line_number":37,"context_line":"class RedisJobboardTest(test.TestCase, base.BoardTestMixin):"},{"line_number":38,"context_line":"    def close_client(self, client):"},{"line_number":39,"context_line":"        client.close()"}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_c95dcc7a","line":36,"updated":"2015-05-02 04:29:57.000000000","message":"why not use fakeredis for testing? (https://pypi.python.org/pypi/fakeredis/0.6.1)","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"5e53126b7b967575f4b8707466f15fc6233c1d37","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    impl_redis.RedisJobBoard.MIN_REDIS_VERSION)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"@testtools.skipIf(not REDIS_AVAILABLE, \u0027redis is not available\u0027)"},{"line_number":37,"context_line":"class RedisJobboardTest(test.TestCase, base.BoardTestMixin):"},{"line_number":38,"context_line":"    def close_client(self, client):"},{"line_number":39,"context_line":"        client.close()"}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_89ac7436","line":36,"in_reply_to":"3acd31a7_c95dcc7a","updated":"2015-05-02 05:23:32.000000000","message":"Looked into that, it appears only https://github.com/locationlabs/mockredis/  has any sort of lua support (although not full support for what we need/use), so until that library (or fakeredis) gets more capable we\u0027ll just have to run against a real redis server (the openstack-infra VM where this unit tests run has redis running, and it\u0027s a new enough version). Also redis 2.6 or newer (the one with lua support) is pretty much available everywhere so shouldn\u0027t be to much to require this (until the above mocking libraries are improved).","commit_id":"fd875c6d16707dcf8cb1c2304ed2cac8208452d7"}]}