{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/8672","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/8672/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/8672/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/8672/events","html_url":"https://github.com/cockroachdb/cockroach/issues/8672","id":171994408,"number":8672,"title":"storage: consider returning Raft errors across RaftTransport to the initiating store","user":{"login":"arjunravinarayan","id":1565833,"avatar_url":"https://avatars.githubusercontent.com/u/1565833?v=3","gravatar_id":"","url":"https://api.github.com/users/arjunravinarayan","html_url":"https://github.com/arjunravinarayan","followers_url":"https://api.github.com/users/arjunravinarayan/followers","following_url":"https://api.github.com/users/arjunravinarayan/following{/other_user}","gists_url":"https://api.github.com/users/arjunravinarayan/gists{/gist_id}","starred_url":"https://api.github.com/users/arjunravinarayan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/arjunravinarayan/subscriptions","organizations_url":"https://api.github.com/users/arjunravinarayan/orgs","repos_url":"https://api.github.com/users/arjunravinarayan/repos","events_url":"https://api.github.com/users/arjunravinarayan/events{/privacy}","received_events_url":"https://api.github.com/users/arjunravinarayan/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/cockroachdb/cockroach/labels/cleanup","name":"cleanup","color":"fef2c0"},{"url":"https://api.github.com/repos/cockroachdb/cockroach/labels/helpwanted","name":"helpwanted","color":"bfdadc"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/cockroachdb/cockroach/milestones/3","html_url":"https://github.com/cockroachdb/cockroach/milestones/Later","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/milestones/3/labels","id":1298443,"number":3,"title":"Later","description":"","creator":{"login":"BramGruneir","id":1614265,"avatar_url":"https://avatars.githubusercontent.com/u/1614265?v=3","gravatar_id":"","url":"https://api.github.com/users/BramGruneir","html_url":"https://github.com/BramGruneir","followers_url":"https://api.github.com/users/BramGruneir/followers","following_url":"https://api.github.com/users/BramGruneir/following{/other_user}","gists_url":"https://api.github.com/users/BramGruneir/gists{/gist_id}","starred_url":"https://api.github.com/users/BramGruneir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BramGruneir/subscriptions","organizations_url":"https://api.github.com/users/BramGruneir/orgs","repos_url":"https://api.github.com/users/BramGruneir/repos","events_url":"https://api.github.com/users/BramGruneir/events{/privacy}","received_events_url":"https://api.github.com/users/BramGruneir/received_events","type":"User","site_admin":false},"open_issues":39,"closed_issues":2,"state":"open","created_at":"2015-09-09T21:24:17Z","updated_at":"2016-08-29T21:59:37Z","due_on":null,"closed_at":null},"comments":2,"created_at":"2016-08-18T20:34:39Z","updated_at":"2016-09-01T07:27:08Z","closed_at":null,"body":"#8495 changed `RaftTransport` to allow for errors to be returned across the network to remote nodes without shutting down the entire stream. Some more elegant behavior is now possible - for instance, we currently silently drop snapshot messages ([`store.go:2198`](https://github.com/cockroachdb/cockroach/blob/master/storage/store.go#L2198) in`HandleRaftRequest`) if they fail to be ratified by `canApplySnapshotLocked`. Now that we can return errors across nodes, it might be worth returning an error in that situation, although it is unclear to me whether any useful behavioral change would result on the initiating store besides better logging."},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/243998062","html_url":"https://github.com/cockroachdb/cockroach/issues/8672#issuecomment-243998062","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/8672","id":243998062,"user":{"login":"bdarnell","id":160562,"avatar_url":"https://avatars.githubusercontent.com/u/160562?v=3","gravatar_id":"","url":"https://api.github.com/users/bdarnell","html_url":"https://github.com/bdarnell","followers_url":"https://api.github.com/users/bdarnell/followers","following_url":"https://api.github.com/users/bdarnell/following{/other_user}","gists_url":"https://api.github.com/users/bdarnell/gists{/gist_id}","starred_url":"https://api.github.com/users/bdarnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bdarnell/subscriptions","organizations_url":"https://api.github.com/users/bdarnell/orgs","repos_url":"https://api.github.com/users/bdarnell/repos","events_url":"https://api.github.com/users/bdarnell/events{/privacy}","received_events_url":"https://api.github.com/users/bdarnell/received_events","type":"User","site_admin":false},"created_at":"2016-09-01T07:27:07Z","updated_at":"2016-09-01T07:27:07Z","body":"@tamird What kind of correctness issues could be masked? It takes more than vague FUD to make this a priority.\r\n\r\nIn the case of the `canApplySnapshot` check, I think returning an error here is best thought of as an optimization: If `canApplySnapshot` fails, this is a condition that will not be resolved by simply retrying, so we don't want the sender to think the message was dropped by the network. We should return an error here so that the sender can stop retrying (and initiate a ChangeReplicas to remove the node that is unable to accept a snapshot).\r\n\r\nThis is something that would be useful to us right now on the test clusters; we see some issues where snapshots can't be applied and ranges are stuck (#8942, #8007). But improving error handling here mainly papers over the problem - we should figure out why we're unable to apply the snapshot in the first place (#8944 fixes one case). \r\n\r\nSo this is higher priority than a lot of the stuff in \"Later\", but it's still not something I'd prioritize right now unless we see more evidence that this is a problem in practice."}}
{"actor":{"display_login":"bdarnell"}}