{"action":"created","comment":{"url":"https://api.github.com/repos/citusdata/pg_shard/pulls/comments/33175147","id":33175147,"diff_hunk":"@@ -778,8 +827,10 @@ LockShard(int64 shardId, LOCKMODE lockMode)\n \tmemset(&lockTag, 0, sizeof(LOCKTAG));\n \n \tAssert(lockMode == ExclusiveLock || lockMode == ShareLock);\n+\tAssert(shardLockType = !SHARD_LOCK_INVALID_FIRST);","path":"distribution_metadata.c","position":84,"original_position":84,"commit_id":"884feabefbe0acb5131971c0071512801bf9d3bb","original_commit_id":"884feabefbe0acb5131971c0071512801bf9d3bb","user":{"login":"jasonmp85","id":50673,"avatar_url":"https://avatars.githubusercontent.com/u/50673?v=3","gravatar_id":"","url":"https://api.github.com/users/jasonmp85","html_url":"https://github.com/jasonmp85","followers_url":"https://api.github.com/users/jasonmp85/followers","following_url":"https://api.github.com/users/jasonmp85/following{/other_user}","gists_url":"https://api.github.com/users/jasonmp85/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonmp85/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonmp85/subscriptions","organizations_url":"https://api.github.com/users/jasonmp85/orgs","repos_url":"https://api.github.com/users/jasonmp85/repos","events_url":"https://api.github.com/users/jasonmp85/events{/privacy}","received_events_url":"https://api.github.com/users/jasonmp85/received_events","type":"User","site_admin":false},"body":"Well that's rather bizarre. I must have transposed characters at some point because I'd never have written this. Fixing.","created_at":"2015-06-24T17:49:38Z","updated_at":"2015-06-24T17:49:38Z","html_url":"https://github.com/citusdata/pg_shard/pull/123#discussion_r33175147","pull_request_url":"https://api.github.com/repos/citusdata/pg_shard/pulls/123","_links":{"self":{"href":"https://api.github.com/repos/citusdata/pg_shard/pulls/comments/33175147"},"html":{"href":"https://github.com/citusdata/pg_shard/pull/123#discussion_r33175147"},"pull_request":{"href":"https://api.github.com/repos/citusdata/pg_shard/pulls/123"}}},"pull_request":{"url":"https://api.github.com/repos/citusdata/pg_shard/pulls/123","id":38021038,"html_url":"https://github.com/citusdata/pg_shard/pull/123","diff_url":"https://github.com/citusdata/pg_shard/pull/123.diff","patch_url":"https://github.com/citusdata/pg_shard/pull/123.patch","issue_url":"https://api.github.com/repos/citusdata/pg_shard/issues/123","number":123,"state":"open","locked":false,"title":"Lock metadata during cluster structural changes","user":{"login":"jasonmp85","id":50673,"avatar_url":"https://avatars.githubusercontent.com/u/50673?v=3","gravatar_id":"","url":"https://api.github.com/users/jasonmp85","html_url":"https://github.com/jasonmp85","followers_url":"https://api.github.com/users/jasonmp85/followers","following_url":"https://api.github.com/users/jasonmp85/following{/other_user}","gists_url":"https://api.github.com/users/jasonmp85/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonmp85/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonmp85/subscriptions","organizations_url":"https://api.github.com/users/jasonmp85/orgs","repos_url":"https://api.github.com/users/jasonmp85/repos","events_url":"https://api.github.com/users/jasonmp85/events{/privacy}","received_events_url":"https://api.github.com/users/jasonmp85/received_events","type":"User","site_admin":false},"body":"The guiding principle here is that readers must know about a relevant set of objects to perform distributed queries so need guarantees that the relevant set will not change beneath them during said queries.\n\nIn practice, this means we need two locks: one for the set of shards related to a relation and one for the set of placements related to a shard. If the caller intends to add shards (or (active) placements), they must hold an exclusive lock on the grouping relation (or shard). If the caller just wants to act on the current set of shards (or the (active) placements), they need a shared lock before reading.\n\nI explored using row locks to accomplish this but they'd be just as arbitrary as some advisory lock with the added downside that they can cause disk writes. `LockDatabaseObject` promised to provide a more structured way to specify what we're locking, but it is usually called with a `relid`-`objid` pair, each half of which is 32-bits. This provides two problems:\n\n  * The adapter layer between `pg_shard` and CitusDB's metadata is implemented with a view, so we couldn't blindly use a `relid`: we'd have to get the `relid` of the underlying query if the metadata table in question is a view instead of a normal relation. This is a bit of extra code.\n  * Shard identifiers are 64-bits and so do not fit in the space afforded by a call to `LockDatabaseObject` (assuming the first parameter is a `relid`). We could has the shard identifier to fit in 32-bits, but that's again extra code\n\nUltimately I decided just using an `AccessExclusive` lock on the relation itself was sufficient to signal we're reading or changing that relation's shard set (use of an `AccessExclusive` lock here is analogous to PostgreSQL's own use of that level for most table schema changes). The lock used to read or change placements of a shard is an advisory lock comprised of the database identifier, the shard identifier, and a lock type (metadata or actual data).","created_at":"2015-06-18T15:41:57Z","updated_at":"2015-06-24T17:49:38Z","closed_at":null,"merged_at":null,"merge_commit_sha":"63db3dad505fe7f15b4ec7703bce3678f2e5be93","assignee":{"login":"anarazel","id":1598211,"avatar_url":"https://avatars.githubusercontent.com/u/1598211?v=3","gravatar_id":"","url":"https://api.github.com/users/anarazel","html_url":"https://github.com/anarazel","followers_url":"https://api.github.com/users/anarazel/followers","following_url":"https://api.github.com/users/anarazel/following{/other_user}","gists_url":"https://api.github.com/users/anarazel/gists{/gist_id}","starred_url":"https://api.github.com/users/anarazel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anarazel/subscriptions","organizations_url":"https://api.github.com/users/anarazel/orgs","repos_url":"https://api.github.com/users/anarazel/repos","events_url":"https://api.github.com/users/anarazel/events{/privacy}","received_events_url":"https://api.github.com/users/anarazel/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/citusdata/pg_shard/pulls/123/commits","review_comments_url":"https://api.github.com/repos/citusdata/pg_shard/pulls/123/comments","review_comment_url":"https://api.github.com/repos/citusdata/pg_shard/pulls/comments{/number}","comments_url":"https://api.github.com/repos/citusdata/pg_shard/issues/123/comments","statuses_url":"https://api.github.com/repos/citusdata/pg_shard/statuses/884feabefbe0acb5131971c0071512801bf9d3bb","head":{"label":"citusdata:feature-metadata_locks","ref":"feature-metadata_locks","sha":"884feabefbe0acb5131971c0071512801bf9d3bb","user":{"login":"citusdata","id":2545424,"avatar_url":"https://avatars.githubusercontent.com/u/2545424?v=3","gravatar_id":"","url":"https://api.github.com/users/citusdata","html_url":"https://github.com/citusdata","followers_url":"https://api.github.com/users/citusdata/followers","following_url":"https://api.github.com/users/citusdata/following{/other_user}","gists_url":"https://api.github.com/users/citusdata/gists{/gist_id}","starred_url":"https://api.github.com/users/citusdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citusdata/subscriptions","organizations_url":"https://api.github.com/users/citusdata/orgs","repos_url":"https://api.github.com/users/citusdata/repos","events_url":"https://api.github.com/users/citusdata/events{/privacy}","received_events_url":"https://api.github.com/users/citusdata/received_events","type":"Organization","site_admin":false},"repo":{"id":27522888,"name":"pg_shard","full_name":"citusdata/pg_shard","owner":{"login":"citusdata","id":2545424,"avatar_url":"https://avatars.githubusercontent.com/u/2545424?v=3","gravatar_id":"","url":"https://api.github.com/users/citusdata","html_url":"https://github.com/citusdata","followers_url":"https://api.github.com/users/citusdata/followers","following_url":"https://api.github.com/users/citusdata/following{/other_user}","gists_url":"https://api.github.com/users/citusdata/gists{/gist_id}","starred_url":"https://api.github.com/users/citusdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citusdata/subscriptions","organizations_url":"https://api.github.com/users/citusdata/orgs","repos_url":"https://api.github.com/users/citusdata/repos","events_url":"https://api.github.com/users/citusdata/events{/privacy}","received_events_url":"https://api.github.com/users/citusdata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/citusdata/pg_shard","description":"PostgreSQL extension to scale out real-time reads and writes. Check out the mailing list at https://groups.google.com/group/pg_shard-users or read the docs at http://citusdata.com/docs/pg-shard","fork":false,"url":"https://api.github.com/repos/citusdata/pg_shard","forks_url":"https://api.github.com/repos/citusdata/pg_shard/forks","keys_url":"https://api.github.com/repos/citusdata/pg_shard/keys{/key_id}","collaborators_url":"https://api.github.com/repos/citusdata/pg_shard/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/citusdata/pg_shard/teams","hooks_url":"https://api.github.com/repos/citusdata/pg_shard/hooks","issue_events_url":"https://api.github.com/repos/citusdata/pg_shard/issues/events{/number}","events_url":"https://api.github.com/repos/citusdata/pg_shard/events","assignees_url":"https://api.github.com/repos/citusdata/pg_shard/assignees{/user}","branches_url":"https://api.github.com/repos/citusdata/pg_shard/branches{/branch}","tags_url":"https://api.github.com/repos/citusdata/pg_shard/tags","blobs_url":"https://api.github.com/repos/citusdata/pg_shard/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/citusdata/pg_shard/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/citusdata/pg_shard/git/refs{/sha}","trees_url":"https://api.github.com/repos/citusdata/pg_shard/git/trees{/sha}","statuses_url":"https://api.github.com/repos/citusdata/pg_shard/statuses/{sha}","languages_url":"https://api.github.com/repos/citusdata/pg_shard/languages","stargazers_url":"https://api.github.com/repos/citusdata/pg_shard/stargazers","contributors_url":"https://api.github.com/repos/citusdata/pg_shard/contributors","subscribers_url":"https://api.github.com/repos/citusdata/pg_shard/subscribers","subscription_url":"https://api.github.com/repos/citusdata/pg_shard/subscription","commits_url":"https://api.github.com/repos/citusdata/pg_shard/commits{/sha}","git_commits_url":"https://api.github.com/repos/citusdata/pg_shard/git/commits{/sha}","comments_url":"https://api.github.com/repos/citusdata/pg_shard/comments{/number}","issue_comment_url":"https://api.github.com/repos/citusdata/pg_shard/issues/comments{/number}","contents_url":"https://api.github.com/repos/citusdata/pg_shard/contents/{+path}","compare_url":"https://api.github.com/repos/citusdata/pg_shard/compare/{base}...{head}","merges_url":"https://api.github.com/repos/citusdata/pg_shard/merges","archive_url":"https://api.github.com/repos/citusdata/pg_shard/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/citusdata/pg_shard/downloads","issues_url":"https://api.github.com/repos/citusdata/pg_shard/issues{/number}","pulls_url":"https://api.github.com/repos/citusdata/pg_shard/pulls{/number}","milestones_url":"https://api.github.com/repos/citusdata/pg_shard/milestones{/number}","notifications_url":"https://api.github.com/repos/citusdata/pg_shard/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/citusdata/pg_shard/labels{/name}","releases_url":"https://api.github.com/repos/citusdata/pg_shard/releases{/id}","created_at":"2014-12-04T04:24:37Z","updated_at":"2015-06-23T22:33:12Z","pushed_at":"2015-06-24T14:25:14Z","git_url":"git://github.com/citusdata/pg_shard.git","ssh_url":"git@github.com:citusdata/pg_shard.git","clone_url":"https://github.com/citusdata/pg_shard.git","svn_url":"https://github.com/citusdata/pg_shard","homepage":"","size":2819,"stargazers_count":748,"watchers_count":748,"language":"C","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":31,"mirror_url":null,"open_issues_count":45,"forks":31,"open_issues":45,"watchers":748,"default_branch":"develop"}},"base":{"label":"citusdata:develop","ref":"develop","sha":"e0aaa06d8a613119a6037a74b177fa877d3c04bc","user":{"login":"citusdata","id":2545424,"avatar_url":"https://avatars.githubusercontent.com/u/2545424?v=3","gravatar_id":"","url":"https://api.github.com/users/citusdata","html_url":"https://github.com/citusdata","followers_url":"https://api.github.com/users/citusdata/followers","following_url":"https://api.github.com/users/citusdata/following{/other_user}","gists_url":"https://api.github.com/users/citusdata/gists{/gist_id}","starred_url":"https://api.github.com/users/citusdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citusdata/subscriptions","organizations_url":"https://api.github.com/users/citusdata/orgs","repos_url":"https://api.github.com/users/citusdata/repos","events_url":"https://api.github.com/users/citusdata/events{/privacy}","received_events_url":"https://api.github.com/users/citusdata/received_events","type":"Organization","site_admin":false},"repo":{"id":27522888,"name":"pg_shard","full_name":"citusdata/pg_shard","owner":{"login":"citusdata","id":2545424,"avatar_url":"https://avatars.githubusercontent.com/u/2545424?v=3","gravatar_id":"","url":"https://api.github.com/users/citusdata","html_url":"https://github.com/citusdata","followers_url":"https://api.github.com/users/citusdata/followers","following_url":"https://api.github.com/users/citusdata/following{/other_user}","gists_url":"https://api.github.com/users/citusdata/gists{/gist_id}","starred_url":"https://api.github.com/users/citusdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citusdata/subscriptions","organizations_url":"https://api.github.com/users/citusdata/orgs","repos_url":"https://api.github.com/users/citusdata/repos","events_url":"https://api.github.com/users/citusdata/events{/privacy}","received_events_url":"https://api.github.com/users/citusdata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/citusdata/pg_shard","description":"PostgreSQL extension to scale out real-time reads and writes. Check out the mailing list at https://groups.google.com/group/pg_shard-users or read the docs at http://citusdata.com/docs/pg-shard","fork":false,"url":"https://api.github.com/repos/citusdata/pg_shard","forks_url":"https://api.github.com/repos/citusdata/pg_shard/forks","keys_url":"https://api.github.com/repos/citusdata/pg_shard/keys{/key_id}","collaborators_url":"https://api.github.com/repos/citusdata/pg_shard/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/citusdata/pg_shard/teams","hooks_url":"https://api.github.com/repos/citusdata/pg_shard/hooks","issue_events_url":"https://api.github.com/repos/citusdata/pg_shard/issues/events{/number}","events_url":"https://api.github.com/repos/citusdata/pg_shard/events","assignees_url":"https://api.github.com/repos/citusdata/pg_shard/assignees{/user}","branches_url":"https://api.github.com/repos/citusdata/pg_shard/branches{/branch}","tags_url":"https://api.github.com/repos/citusdata/pg_shard/tags","blobs_url":"https://api.github.com/repos/citusdata/pg_shard/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/citusdata/pg_shard/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/citusdata/pg_shard/git/refs{/sha}","trees_url":"https://api.github.com/repos/citusdata/pg_shard/git/trees{/sha}","statuses_url":"https://api.github.com/repos/citusdata/pg_shard/statuses/{sha}","languages_url":"https://api.github.com/repos/citusdata/pg_shard/languages","stargazers_url":"https://api.github.com/repos/citusdata/pg_shard/stargazers","contributors_url":"https://api.github.com/repos/citusdata/pg_shard/contributors","subscribers_url":"https://api.github.com/repos/citusdata/pg_shard/subscribers","subscription_url":"https://api.github.com/repos/citusdata/pg_shard/subscription","commits_url":"https://api.github.com/repos/citusdata/pg_shard/commits{/sha}","git_commits_url":"https://api.github.com/repos/citusdata/pg_shard/git/commits{/sha}","comments_url":"https://api.github.com/repos/citusdata/pg_shard/comments{/number}","issue_comment_url":"https://api.github.com/repos/citusdata/pg_shard/issues/comments{/number}","contents_url":"https://api.github.com/repos/citusdata/pg_shard/contents/{+path}","compare_url":"https://api.github.com/repos/citusdata/pg_shard/compare/{base}...{head}","merges_url":"https://api.github.com/repos/citusdata/pg_shard/merges","archive_url":"https://api.github.com/repos/citusdata/pg_shard/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/citusdata/pg_shard/downloads","issues_url":"https://api.github.com/repos/citusdata/pg_shard/issues{/number}","pulls_url":"https://api.github.com/repos/citusdata/pg_shard/pulls{/number}","milestones_url":"https://api.github.com/repos/citusdata/pg_shard/milestones{/number}","notifications_url":"https://api.github.com/repos/citusdata/pg_shard/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/citusdata/pg_shard/labels{/name}","releases_url":"https://api.github.com/repos/citusdata/pg_shard/releases{/id}","created_at":"2014-12-04T04:24:37Z","updated_at":"2015-06-23T22:33:12Z","pushed_at":"2015-06-24T14:25:14Z","git_url":"git://github.com/citusdata/pg_shard.git","ssh_url":"git@github.com:citusdata/pg_shard.git","clone_url":"https://github.com/citusdata/pg_shard.git","svn_url":"https://github.com/citusdata/pg_shard","homepage":"","size":2819,"stargazers_count":748,"watchers_count":748,"language":"C","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":31,"mirror_url":null,"open_issues_count":45,"forks":31,"open_issues":45,"watchers":748,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/citusdata/pg_shard/pulls/123"},"html":{"href":"https://github.com/citusdata/pg_shard/pull/123"},"issue":{"href":"https://api.github.com/repos/citusdata/pg_shard/issues/123"},"comments":{"href":"https://api.github.com/repos/citusdata/pg_shard/issues/123/comments"},"review_comments":{"href":"https://api.github.com/repos/citusdata/pg_shard/pulls/123/comments"},"review_comment":{"href":"https://api.github.com/repos/citusdata/pg_shard/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/citusdata/pg_shard/pulls/123/commits"},"statuses":{"href":"https://api.github.com/repos/citusdata/pg_shard/statuses/884feabefbe0acb5131971c0071512801bf9d3bb"}}}}