{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1044","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1044/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1044/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1044/events","html_url":"https://github.com/cockroachdb/cockroach/pull/1044","id":76984159,"number":1044,"title":"Do not panic when the range GC queue cannot find a replica of a range","user":{"login":"kkaneda","id":8353550,"avatar_url":"https://avatars.githubusercontent.com/u/8353550?v=3","gravatar_id":"","url":"https://api.github.com/users/kkaneda","html_url":"https://github.com/kkaneda","followers_url":"https://api.github.com/users/kkaneda/followers","following_url":"https://api.github.com/users/kkaneda/following{/other_user}","gists_url":"https://api.github.com/users/kkaneda/gists{/gist_id}","starred_url":"https://api.github.com/users/kkaneda/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kkaneda/subscriptions","organizations_url":"https://api.github.com/users/kkaneda/orgs","repos_url":"https://api.github.com/users/kkaneda/repos","events_url":"https://api.github.com/users/kkaneda/events{/privacy}","received_events_url":"https://api.github.com/users/kkaneda/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/cockroachdb/cockroach/labels/PTAL","name":"PTAL","color":"ededed"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-05-16T08:38:23Z","updated_at":"2015-05-18T13:39:11Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1044","html_url":"https://github.com/cockroachdb/cockroach/pull/1044","diff_url":"https://github.com/cockroachdb/cockroach/pull/1044.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/1044.patch"},"body":"This might happen only in tests, but the following test caused a panic if the range GC queue tries to process the bootstrap range while `replicateRange()` is called.\r\n\r\n```\r\nfunc TestRangeGCQueue(t *testing.T) {\r\n     defer leaktest.AfterTest(t)\r\n     mtc := &multiTestContext{}\r\n     mtc.Start(t, 3)\r\n     defer mtc.Stop()\r\n     raftID := int64(1)\r\n     mtc.replicateRange(raftID, 0, 1, 2)\r\n}\r\n```\r\n\r\nThe panic message is following:\r\n\r\n```\r\nL=\"FATAL\" T=\"5/16 01:24:02.783\" F=\"range.go:442\" Msg=\"own replica missing in range at store 3\"\r\n```\r\n\r\nHere is what happened (I think):\r\n\r\n1) `replicateRange()` calls `ChangeReplicas()`, which starts a txn to update the range descriptor.\r\n2) Store 3 receives a Raft message from an unknown Raft group (ID 1).\r\n3) Store 3 creates a new range with Raft ID 1 via `GroupStorage()`. The method does not populate a replica list.\r\n4) Before the txn is committed and the range's replica list is updated, the range GC queue tries to process the range.\r\n5) `GetReplica()` causes a panic since no replica is found.\r\n\r\nAlternative solutions:\r\n- Make `GroupStorage()` somehow populate a replica list.\r\n- Disable the scanner in `multiTestContext`."},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/103062708","html_url":"https://github.com/cockroachdb/cockroach/pull/1044#issuecomment-103062708","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1044","id":103062708,"user":{"login":"tschottdorf","id":5076964,"avatar_url":"https://avatars.githubusercontent.com/u/5076964?v=3","gravatar_id":"","url":"https://api.github.com/users/tschottdorf","html_url":"https://github.com/tschottdorf","followers_url":"https://api.github.com/users/tschottdorf/followers","following_url":"https://api.github.com/users/tschottdorf/following{/other_user}","gists_url":"https://api.github.com/users/tschottdorf/gists{/gist_id}","starred_url":"https://api.github.com/users/tschottdorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tschottdorf/subscriptions","organizations_url":"https://api.github.com/users/tschottdorf/orgs","repos_url":"https://api.github.com/users/tschottdorf/repos","events_url":"https://api.github.com/users/tschottdorf/events{/privacy}","received_events_url":"https://api.github.com/users/tschottdorf/received_events","type":"User","site_admin":false},"created_at":"2015-05-18T13:39:11Z","updated_at":"2015-05-18T13:39:11Z","body":"It seems somehow reasonable that `GroupStorage()` shouldn't pass out an empty replica list given that it knows that the requested range should be in it (?). I'm not sure about the implications though, and it might not matter if the naive solution is the correct one: the range gc queue should check `r.isInitialized()` (which is only true after a snapshot has been applied, which in turn updates the descriptor accordingly). That of course raises the question of when ranges which were added at some point but then never received a snapshot are garbage collected.\r\n\r\nIt seems awkward to have `GetReplica` and `GetReplicaOptional`. Either it should return an error in general or panic, depending on how we go about the above. From the way the GC queue uses it, an error seems appropriate."}}