{"action":"created","comment":{"url":"https://api.github.com/repos/cequel/cequel/pulls/comments/93722184","pull_request_review_id":14255893,"id":93722184,"diff_hunk":"@@ -0,0 +1,79 @@\n+# -*- encoding : utf-8 -*-\n+module Cequel\n+  module Metal\n+    module Policy\n+      module CassandraError\n+        class ErrorPolicyBase                  \n+          # This class is used by the Keyspace object to dictate \n+          # how a failure from Cassandra is handled.\n+          # The only method defined is \n+          #   handle_error(keyspace, error, retries_remaining)\n+          # The first argument is an instance of the Keyspace class\n+          # The second argument is the error from the Cassandra gem.\n+          # The thid argument is the number of remaining retries.\n+          # This function must raise the error to abort the operation,\n+          # if this function returns it indicates the operation should be \n+          # retried.\n+          # \n+          # The specific instance is chosen by passing configuration options\n+          # See Keyspace#configure\n+          \n+          # On instantiation, the configuraiton hash passed to Cequel is \n+          # available here\n+          def initialize(options = {})\n+          end\n+                    \n+          def handle_error(error, retries_remaining)\n+            raise RuntimeError, 'This is an abstract base class, never call this'\n+          end","path":"lib/cequel/metal/policy/cassandra_error.rb","position":28,"original_position":28,"commit_id":"3b754b6067013d7fe3bcb39fbd538289ae37396f","original_commit_id":"3b754b6067013d7fe3bcb39fbd538289ae37396f","user":{"login":"pezra","id":4122,"avatar_url":"https://avatars.githubusercontent.com/u/4122?v=3","gravatar_id":"","url":"https://api.github.com/users/pezra","html_url":"https://github.com/pezra","followers_url":"https://api.github.com/users/pezra/followers","following_url":"https://api.github.com/users/pezra/following{/other_user}","gists_url":"https://api.github.com/users/pezra/gists{/gist_id}","starred_url":"https://api.github.com/users/pezra/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pezra/subscriptions","organizations_url":"https://api.github.com/users/pezra/orgs","repos_url":"https://api.github.com/users/pezra/repos","events_url":"https://api.github.com/users/pezra/events{/privacy}","received_events_url":"https://api.github.com/users/pezra/received_events","type":"User","site_admin":false},"body":"I think the above method should be replaced with\r\n```ruby\r\ndef execute_stmt(keyspace)\r\n  raise NotImplementedError, \"#execute_stmt must be implemented in #{self.class}\"\r\nend\r\n```","created_at":"2016-12-23T01:31:00Z","updated_at":"2016-12-23T01:45:14Z","html_url":"https://github.com/cequel/cequel/pull/336#discussion_r93722184","pull_request_url":"https://api.github.com/repos/cequel/cequel/pulls/336","_links":{"self":{"href":"https://api.github.com/repos/cequel/cequel/pulls/comments/93722184"},"html":{"href":"https://github.com/cequel/cequel/pull/336#discussion_r93722184"},"pull_request":{"href":"https://api.github.com/repos/cequel/cequel/pulls/336"}}},"pull_request":{"url":"https://api.github.com/repos/cequel/cequel/pulls/336","id":91809015,"html_url":"https://github.com/cequel/cequel/pull/336","diff_url":"https://github.com/cequel/cequel/pull/336.diff","patch_url":"https://github.com/cequel/cequel/pull/336.patch","issue_url":"https://api.github.com/repos/cequel/cequel/issues/336","number":336,"state":"open","locked":false,"title":"Delegate error handling to a policy object, allow for arbitrary options to be passed to cassandra driver gem","user":{"login":"hydrogen18","id":130230,"avatar_url":"https://avatars.githubusercontent.com/u/130230?v=3","gravatar_id":"","url":"https://api.github.com/users/hydrogen18","html_url":"https://github.com/hydrogen18","followers_url":"https://api.github.com/users/hydrogen18/followers","following_url":"https://api.github.com/users/hydrogen18/following{/other_user}","gists_url":"https://api.github.com/users/hydrogen18/gists{/gist_id}","starred_url":"https://api.github.com/users/hydrogen18/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hydrogen18/subscriptions","organizations_url":"https://api.github.com/users/hydrogen18/orgs","repos_url":"https://api.github.com/users/hydrogen18/repos","events_url":"https://api.github.com/users/hydrogen18/events{/privacy}","received_events_url":"https://api.github.com/users/hydrogen18/received_events","type":"User","site_admin":false},"body":"After refactoring some of the keyspace object, I can't really figure out why you would want to call `clear_active_connections!` after each error. This may have worked around some shortcomings in the cassandra driver gem historically, but I don't think it is a very good idea. The cassandra driver gem supports an infinite amount of configuration in the form of policy objects, which can be used to implement whatever handling logic you would like. It also tracks what hosts are up and down, so there is little reason to clear the connections on an error. I also can't see why you would to do retries in this gem, given that the cassandra driver gem can do that for you.\r\n\r\nWhat I've seen in production systems is that once our application encounters a single timeout, all application threads immediately start hammering all the nodes. I think this is because clearing the connections makes the driver gem \"forget\" which hosts are up and down. \r\n\r\nAll that taken into consideration, I don't want to change the default behavior because that would make the version '3.0.0' basically. So instead I have added two options to the configuration hash passed to this gem\r\n\r\n1. cassandra_error_policy\r\n2. cassandra_options\r\n\r\nUsing these two options, you can disable the default behavior and pass whatever options you would like to the underlying driver gem. Specifically, specifying a behavior of `Cequel::Metal::Keyspace::Policy::CassandraError::Raise` results in errors from the cassandra driver gem simply propagating up the call stack.\r\n\r\nLet me know if you need anything else to accept this merge.","created_at":"2016-11-01T15:53:17Z","updated_at":"2016-12-23T01:45:14Z","closed_at":null,"merged_at":null,"merge_commit_sha":"9f52792d6c093096224255b70728f3ea494f387d","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/cequel/cequel/pulls/336/commits","review_comments_url":"https://api.github.com/repos/cequel/cequel/pulls/336/comments","review_comment_url":"https://api.github.com/repos/cequel/cequel/pulls/comments{/number}","comments_url":"https://api.github.com/repos/cequel/cequel/issues/336/comments","statuses_url":"https://api.github.com/repos/cequel/cequel/statuses/3b754b6067013d7fe3bcb39fbd538289ae37396f","head":{"label":"spiceworks:dont-drop-conns","ref":"dont-drop-conns","sha":"3b754b6067013d7fe3bcb39fbd538289ae37396f","user":{"login":"spiceworks","id":60843,"avatar_url":"https://avatars.githubusercontent.com/u/60843?v=3","gravatar_id":"","url":"https://api.github.com/users/spiceworks","html_url":"https://github.com/spiceworks","followers_url":"https://api.github.com/users/spiceworks/followers","following_url":"https://api.github.com/users/spiceworks/following{/other_user}","gists_url":"https://api.github.com/users/spiceworks/gists{/gist_id}","starred_url":"https://api.github.com/users/spiceworks/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spiceworks/subscriptions","organizations_url":"https://api.github.com/users/spiceworks/orgs","repos_url":"https://api.github.com/users/spiceworks/repos","events_url":"https://api.github.com/users/spiceworks/events{/privacy}","received_events_url":"https://api.github.com/users/spiceworks/received_events","type":"Organization","site_admin":false},"repo":{"id":66856324,"name":"cequel","full_name":"spiceworks/cequel","owner":{"login":"spiceworks","id":60843,"avatar_url":"https://avatars.githubusercontent.com/u/60843?v=3","gravatar_id":"","url":"https://api.github.com/users/spiceworks","html_url":"https://github.com/spiceworks","followers_url":"https://api.github.com/users/spiceworks/followers","following_url":"https://api.github.com/users/spiceworks/following{/other_user}","gists_url":"https://api.github.com/users/spiceworks/gists{/gist_id}","starred_url":"https://api.github.com/users/spiceworks/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spiceworks/subscriptions","organizations_url":"https://api.github.com/users/spiceworks/orgs","repos_url":"https://api.github.com/users/spiceworks/repos","events_url":"https://api.github.com/users/spiceworks/events{/privacy}","received_events_url":"https://api.github.com/users/spiceworks/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/spiceworks/cequel","description":"Ruby ORM for Cassandra with CQL3","fork":true,"url":"https://api.github.com/repos/spiceworks/cequel","forks_url":"https://api.github.com/repos/spiceworks/cequel/forks","keys_url":"https://api.github.com/repos/spiceworks/cequel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/spiceworks/cequel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/spiceworks/cequel/teams","hooks_url":"https://api.github.com/repos/spiceworks/cequel/hooks","issue_events_url":"https://api.github.com/repos/spiceworks/cequel/issues/events{/number}","events_url":"https://api.github.com/repos/spiceworks/cequel/events","assignees_url":"https://api.github.com/repos/spiceworks/cequel/assignees{/user}","branches_url":"https://api.github.com/repos/spiceworks/cequel/branches{/branch}","tags_url":"https://api.github.com/repos/spiceworks/cequel/tags","blobs_url":"https://api.github.com/repos/spiceworks/cequel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/spiceworks/cequel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/spiceworks/cequel/git/refs{/sha}","trees_url":"https://api.github.com/repos/spiceworks/cequel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/spiceworks/cequel/statuses/{sha}","languages_url":"https://api.github.com/repos/spiceworks/cequel/languages","stargazers_url":"https://api.github.com/repos/spiceworks/cequel/stargazers","contributors_url":"https://api.github.com/repos/spiceworks/cequel/contributors","subscribers_url":"https://api.github.com/repos/spiceworks/cequel/subscribers","subscription_url":"https://api.github.com/repos/spiceworks/cequel/subscription","commits_url":"https://api.github.com/repos/spiceworks/cequel/commits{/sha}","git_commits_url":"https://api.github.com/repos/spiceworks/cequel/git/commits{/sha}","comments_url":"https://api.github.com/repos/spiceworks/cequel/comments{/number}","issue_comment_url":"https://api.github.com/repos/spiceworks/cequel/issues/comments{/number}","contents_url":"https://api.github.com/repos/spiceworks/cequel/contents/{+path}","compare_url":"https://api.github.com/repos/spiceworks/cequel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/spiceworks/cequel/merges","archive_url":"https://api.github.com/repos/spiceworks/cequel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/spiceworks/cequel/downloads","issues_url":"https://api.github.com/repos/spiceworks/cequel/issues{/number}","pulls_url":"https://api.github.com/repos/spiceworks/cequel/pulls{/number}","milestones_url":"https://api.github.com/repos/spiceworks/cequel/milestones{/number}","notifications_url":"https://api.github.com/repos/spiceworks/cequel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/spiceworks/cequel/labels{/name}","releases_url":"https://api.github.com/repos/spiceworks/cequel/releases{/id}","deployments_url":"https://api.github.com/repos/spiceworks/cequel/deployments","created_at":"2016-08-29T15:34:09Z","updated_at":"2016-08-29T15:34:10Z","pushed_at":"2016-11-28T13:52:38Z","git_url":"git://github.com/spiceworks/cequel.git","ssh_url":"git@github.com:spiceworks/cequel.git","clone_url":"https://github.com/spiceworks/cequel.git","svn_url":"https://github.com/spiceworks/cequel","homepage":"","size":1661,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"cequel:master","ref":"master","sha":"4a0263ecf2e6c05653c1febc3066cbdd262c9dd3","user":{"login":"cequel","id":5330342,"avatar_url":"https://avatars.githubusercontent.com/u/5330342?v=3","gravatar_id":"","url":"https://api.github.com/users/cequel","html_url":"https://github.com/cequel","followers_url":"https://api.github.com/users/cequel/followers","following_url":"https://api.github.com/users/cequel/following{/other_user}","gists_url":"https://api.github.com/users/cequel/gists{/gist_id}","starred_url":"https://api.github.com/users/cequel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cequel/subscriptions","organizations_url":"https://api.github.com/users/cequel/orgs","repos_url":"https://api.github.com/users/cequel/repos","events_url":"https://api.github.com/users/cequel/events{/privacy}","received_events_url":"https://api.github.com/users/cequel/received_events","type":"Organization","site_admin":false},"repo":{"id":3929937,"name":"cequel","full_name":"cequel/cequel","owner":{"login":"cequel","id":5330342,"avatar_url":"https://avatars.githubusercontent.com/u/5330342?v=3","gravatar_id":"","url":"https://api.github.com/users/cequel","html_url":"https://github.com/cequel","followers_url":"https://api.github.com/users/cequel/followers","following_url":"https://api.github.com/users/cequel/following{/other_user}","gists_url":"https://api.github.com/users/cequel/gists{/gist_id}","starred_url":"https://api.github.com/users/cequel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cequel/subscriptions","organizations_url":"https://api.github.com/users/cequel/orgs","repos_url":"https://api.github.com/users/cequel/repos","events_url":"https://api.github.com/users/cequel/events{/privacy}","received_events_url":"https://api.github.com/users/cequel/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/cequel/cequel","description":"Ruby ORM for Cassandra with CQL3","fork":false,"url":"https://api.github.com/repos/cequel/cequel","forks_url":"https://api.github.com/repos/cequel/cequel/forks","keys_url":"https://api.github.com/repos/cequel/cequel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cequel/cequel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cequel/cequel/teams","hooks_url":"https://api.github.com/repos/cequel/cequel/hooks","issue_events_url":"https://api.github.com/repos/cequel/cequel/issues/events{/number}","events_url":"https://api.github.com/repos/cequel/cequel/events","assignees_url":"https://api.github.com/repos/cequel/cequel/assignees{/user}","branches_url":"https://api.github.com/repos/cequel/cequel/branches{/branch}","tags_url":"https://api.github.com/repos/cequel/cequel/tags","blobs_url":"https://api.github.com/repos/cequel/cequel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cequel/cequel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cequel/cequel/git/refs{/sha}","trees_url":"https://api.github.com/repos/cequel/cequel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cequel/cequel/statuses/{sha}","languages_url":"https://api.github.com/repos/cequel/cequel/languages","stargazers_url":"https://api.github.com/repos/cequel/cequel/stargazers","contributors_url":"https://api.github.com/repos/cequel/cequel/contributors","subscribers_url":"https://api.github.com/repos/cequel/cequel/subscribers","subscription_url":"https://api.github.com/repos/cequel/cequel/subscription","commits_url":"https://api.github.com/repos/cequel/cequel/commits{/sha}","git_commits_url":"https://api.github.com/repos/cequel/cequel/git/commits{/sha}","comments_url":"https://api.github.com/repos/cequel/cequel/comments{/number}","issue_comment_url":"https://api.github.com/repos/cequel/cequel/issues/comments{/number}","contents_url":"https://api.github.com/repos/cequel/cequel/contents/{+path}","compare_url":"https://api.github.com/repos/cequel/cequel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cequel/cequel/merges","archive_url":"https://api.github.com/repos/cequel/cequel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cequel/cequel/downloads","issues_url":"https://api.github.com/repos/cequel/cequel/issues{/number}","pulls_url":"https://api.github.com/repos/cequel/cequel/pulls{/number}","milestones_url":"https://api.github.com/repos/cequel/cequel/milestones{/number}","notifications_url":"https://api.github.com/repos/cequel/cequel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cequel/cequel/labels{/name}","releases_url":"https://api.github.com/repos/cequel/cequel/releases{/id}","deployments_url":"https://api.github.com/repos/cequel/cequel/deployments","created_at":"2012-04-04T15:07:43Z","updated_at":"2016-12-22T15:21:00Z","pushed_at":"2016-12-23T01:19:10Z","git_url":"git://github.com/cequel/cequel.git","ssh_url":"git@github.com:cequel/cequel.git","clone_url":"https://github.com/cequel/cequel.git","svn_url":"https://github.com/cequel/cequel","homepage":"","size":1874,"stargazers_count":405,"watchers_count":405,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":150,"mirror_url":null,"open_issues_count":59,"forks":150,"open_issues":59,"watchers":405,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/cequel/cequel/pulls/336"},"html":{"href":"https://github.com/cequel/cequel/pull/336"},"issue":{"href":"https://api.github.com/repos/cequel/cequel/issues/336"},"comments":{"href":"https://api.github.com/repos/cequel/cequel/issues/336/comments"},"review_comments":{"href":"https://api.github.com/repos/cequel/cequel/pulls/336/comments"},"review_comment":{"href":"https://api.github.com/repos/cequel/cequel/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/cequel/cequel/pulls/336/commits"},"statuses":{"href":"https://api.github.com/repos/cequel/cequel/statuses/3b754b6067013d7fe3bcb39fbd538289ae37396f"}}}}
{"actor":{"display_login":"pezra"}}