{"action":"created","comment":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments/66658531","id":66658531,"diff_hunk":"@@ -1791,14 +1784,14 @@ AllocSet_GetStats(MemoryContext context, uint64 *nBlocks, uint64 *nChunks,\n     for (block = set->blocks; block != NULL; block = block->next)\n     {\n     \t*nBlocks = *nBlocks + 1;\n-    \tcurrentAllocated += block->endptr - ((char *) block);\n+    \tcurrentAllocated += UserPtr_GetUserPtrSize(block);","path":"src/backend/utils/mmgr/aset.c","position":243,"original_position":243,"commit_id":"c34959fa4b4d9a69b432b7d2c2097b245ae270fa","original_commit_id":"c34959fa4b4d9a69b432b7d2c2097b245ae270fa","user":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"body":"The aset is quite tightly integrated with VMEM already, given that it used to pass the size to gp_free2. As there is no isolation of these two layers of allocators, I don't see any problem with naming the APIs explicitly, instead of more generic names.","created_at":"2016-06-10T18:18:53Z","updated_at":"2016-06-10T18:18:53Z","html_url":"https://github.com/greenplum-db/gpdb/pull/825#discussion_r66658531","pull_request_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825","_links":{"self":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments/66658531"},"html":{"href":"https://github.com/greenplum-db/gpdb/pull/825#discussion_r66658531"},"pull_request":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825"}}},"pull_request":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825","id":72793422,"html_url":"https://github.com/greenplum-db/gpdb/pull/825","diff_url":"https://github.com/greenplum-db/gpdb/pull/825.diff","patch_url":"https://github.com/greenplum-db/gpdb/pull/825.patch","issue_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/825","number":825,"state":"open","locked":false,"title":"Support for size information in Vmem allocator","user":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"body":"### Background\r\n\r\nThe GPDB Vmem is the lowest layer of memory allocator that supports higher allocators such as AllocSet. This layer (mostly defined in memprot.c) is in charge of actually calling malloc/realloc/free to allocate/reallocate/free memory. In this process this layer is also in charge of reserving \"virtual\" memory or Vmem, which is a GPDB specific shared memory counter to track per-segment combined allocations across all the GPDB processes under Vmem umbrella. The Vmem counter is managed by a separate module Vmem_Tracker, and the memprot functions (such as gp_malloc, gp_free2 and gp_realloc) call the APIs provided by VmemTracker.\r\n\r\n### What is this PR accomplishing? \r\n\r\nPreviously the memprot allocators (gp_alloc/gp_realloc/gp_free) were only allocating/freeing memory but were not adding any additional metadata in the header (and there was no header) to track the size of allocations. Therefore, there was no gp_free as freeing memory requires the size of the free to adjust Vmem counter inside VmemTracker. This was patched by explicitly passing size info in gp_free2. \r\n\r\nIn this PR we do the following:\r\n\r\n* We add allocation size in Vmem header (along with checksums which are only available in debug build to detect header and footer boundary, and buffer overruns). \r\n\r\n* We remove size information from the block header of AllocSet. \r\n\r\n* We rename gp_free2 to gp_free as the second parameter (size information) is now obtained from the header and therefore no longer necessary\r\n\r\n* We modify all the consumers of memprot.c APIs to use the new APIs\r\n\r\n* We add unit tests to test the metadata and the correctness of the new Vmem allocators\r\n\r\n\r\n### Why are we making this changes?\r\n\r\nThis is the first step to integrate external modules and third party allocations with Vmem. A long running issue in GPDB is its inability to track allocations by external components including libraries such as ORCA. Therefore, the central Vmem counter is often way off from the underlying allocations, and this may run the system out of memory. By maintaining the size information in the Vmem header, we now have a self-contained allocator that can be exposed to external allocators such as GPOS allocators, without forcing them to manage size information separately.","created_at":"2016-06-06T23:16:23Z","updated_at":"2016-06-10T18:18:53Z","closed_at":null,"merged_at":null,"merge_commit_sha":"38ed7a8ea071f5efc716e5f6178fd61a4f19233b","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825/commits","review_comments_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825/comments","review_comment_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments{/number}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/825/comments","statuses_url":"https://api.github.com/repos/greenplum-db/gpdb/statuses/c34959fa4b4d9a69b432b7d2c2097b245ae270fa","head":{"label":"foyzur:vmem_alloc","ref":"vmem_alloc","sha":"c34959fa4b4d9a69b432b7d2c2097b245ae270fa","user":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"repo":{"id":46012445,"name":"gpdb","full_name":"foyzur/gpdb","owner":{"login":"foyzur","id":1253020,"avatar_url":"https://avatars.githubusercontent.com/u/1253020?v=3","gravatar_id":"","url":"https://api.github.com/users/foyzur","html_url":"https://github.com/foyzur","followers_url":"https://api.github.com/users/foyzur/followers","following_url":"https://api.github.com/users/foyzur/following{/other_user}","gists_url":"https://api.github.com/users/foyzur/gists{/gist_id}","starred_url":"https://api.github.com/users/foyzur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/foyzur/subscriptions","organizations_url":"https://api.github.com/users/foyzur/orgs","repos_url":"https://api.github.com/users/foyzur/repos","events_url":"https://api.github.com/users/foyzur/events{/privacy}","received_events_url":"https://api.github.com/users/foyzur/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/foyzur/gpdb","description":"Pivotal Greenplum Database","fork":true,"url":"https://api.github.com/repos/foyzur/gpdb","forks_url":"https://api.github.com/repos/foyzur/gpdb/forks","keys_url":"https://api.github.com/repos/foyzur/gpdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/foyzur/gpdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/foyzur/gpdb/teams","hooks_url":"https://api.github.com/repos/foyzur/gpdb/hooks","issue_events_url":"https://api.github.com/repos/foyzur/gpdb/issues/events{/number}","events_url":"https://api.github.com/repos/foyzur/gpdb/events","assignees_url":"https://api.github.com/repos/foyzur/gpdb/assignees{/user}","branches_url":"https://api.github.com/repos/foyzur/gpdb/branches{/branch}","tags_url":"https://api.github.com/repos/foyzur/gpdb/tags","blobs_url":"https://api.github.com/repos/foyzur/gpdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/foyzur/gpdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/foyzur/gpdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/foyzur/gpdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/foyzur/gpdb/statuses/{sha}","languages_url":"https://api.github.com/repos/foyzur/gpdb/languages","stargazers_url":"https://api.github.com/repos/foyzur/gpdb/stargazers","contributors_url":"https://api.github.com/repos/foyzur/gpdb/contributors","subscribers_url":"https://api.github.com/repos/foyzur/gpdb/subscribers","subscription_url":"https://api.github.com/repos/foyzur/gpdb/subscription","commits_url":"https://api.github.com/repos/foyzur/gpdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/foyzur/gpdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/foyzur/gpdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/foyzur/gpdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/foyzur/gpdb/contents/{+path}","compare_url":"https://api.github.com/repos/foyzur/gpdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/foyzur/gpdb/merges","archive_url":"https://api.github.com/repos/foyzur/gpdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/foyzur/gpdb/downloads","issues_url":"https://api.github.com/repos/foyzur/gpdb/issues{/number}","pulls_url":"https://api.github.com/repos/foyzur/gpdb/pulls{/number}","milestones_url":"https://api.github.com/repos/foyzur/gpdb/milestones{/number}","notifications_url":"https://api.github.com/repos/foyzur/gpdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/foyzur/gpdb/labels{/name}","releases_url":"https://api.github.com/repos/foyzur/gpdb/releases{/id}","deployments_url":"https://api.github.com/repos/foyzur/gpdb/deployments","created_at":"2015-11-11T21:59:37Z","updated_at":"2016-04-08T17:48:01Z","pushed_at":"2016-06-08T20:05:13Z","git_url":"git://github.com/foyzur/gpdb.git","ssh_url":"git@github.com:foyzur/gpdb.git","clone_url":"https://github.com/foyzur/gpdb.git","svn_url":"https://github.com/foyzur/gpdb","homepage":"http://greenplum.org","size":143181,"stargazers_count":0,"watchers_count":0,"language":"PLpgSQL","has_issues":false,"has_downloads":true,"has_wiki":false,"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":"greenplum-db:master","ref":"master","sha":"1e4947a202ca5db24a20963a2c95698b3be34d93","user":{"login":"greenplum-db","id":14097842,"avatar_url":"https://avatars.githubusercontent.com/u/14097842?v=3","gravatar_id":"","url":"https://api.github.com/users/greenplum-db","html_url":"https://github.com/greenplum-db","followers_url":"https://api.github.com/users/greenplum-db/followers","following_url":"https://api.github.com/users/greenplum-db/following{/other_user}","gists_url":"https://api.github.com/users/greenplum-db/gists{/gist_id}","starred_url":"https://api.github.com/users/greenplum-db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greenplum-db/subscriptions","organizations_url":"https://api.github.com/users/greenplum-db/orgs","repos_url":"https://api.github.com/users/greenplum-db/repos","events_url":"https://api.github.com/users/greenplum-db/events{/privacy}","received_events_url":"https://api.github.com/users/greenplum-db/received_events","type":"Organization","site_admin":false},"repo":{"id":44781140,"name":"gpdb","full_name":"greenplum-db/gpdb","owner":{"login":"greenplum-db","id":14097842,"avatar_url":"https://avatars.githubusercontent.com/u/14097842?v=3","gravatar_id":"","url":"https://api.github.com/users/greenplum-db","html_url":"https://github.com/greenplum-db","followers_url":"https://api.github.com/users/greenplum-db/followers","following_url":"https://api.github.com/users/greenplum-db/following{/other_user}","gists_url":"https://api.github.com/users/greenplum-db/gists{/gist_id}","starred_url":"https://api.github.com/users/greenplum-db/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greenplum-db/subscriptions","organizations_url":"https://api.github.com/users/greenplum-db/orgs","repos_url":"https://api.github.com/users/greenplum-db/repos","events_url":"https://api.github.com/users/greenplum-db/events{/privacy}","received_events_url":"https://api.github.com/users/greenplum-db/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/greenplum-db/gpdb","description":"Greenplum Database","fork":false,"url":"https://api.github.com/repos/greenplum-db/gpdb","forks_url":"https://api.github.com/repos/greenplum-db/gpdb/forks","keys_url":"https://api.github.com/repos/greenplum-db/gpdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/greenplum-db/gpdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/greenplum-db/gpdb/teams","hooks_url":"https://api.github.com/repos/greenplum-db/gpdb/hooks","issue_events_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/events{/number}","events_url":"https://api.github.com/repos/greenplum-db/gpdb/events","assignees_url":"https://api.github.com/repos/greenplum-db/gpdb/assignees{/user}","branches_url":"https://api.github.com/repos/greenplum-db/gpdb/branches{/branch}","tags_url":"https://api.github.com/repos/greenplum-db/gpdb/tags","blobs_url":"https://api.github.com/repos/greenplum-db/gpdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/greenplum-db/gpdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/greenplum-db/gpdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/greenplum-db/gpdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/greenplum-db/gpdb/statuses/{sha}","languages_url":"https://api.github.com/repos/greenplum-db/gpdb/languages","stargazers_url":"https://api.github.com/repos/greenplum-db/gpdb/stargazers","contributors_url":"https://api.github.com/repos/greenplum-db/gpdb/contributors","subscribers_url":"https://api.github.com/repos/greenplum-db/gpdb/subscribers","subscription_url":"https://api.github.com/repos/greenplum-db/gpdb/subscription","commits_url":"https://api.github.com/repos/greenplum-db/gpdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/greenplum-db/gpdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/greenplum-db/gpdb/contents/{+path}","compare_url":"https://api.github.com/repos/greenplum-db/gpdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/greenplum-db/gpdb/merges","archive_url":"https://api.github.com/repos/greenplum-db/gpdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/greenplum-db/gpdb/downloads","issues_url":"https://api.github.com/repos/greenplum-db/gpdb/issues{/number}","pulls_url":"https://api.github.com/repos/greenplum-db/gpdb/pulls{/number}","milestones_url":"https://api.github.com/repos/greenplum-db/gpdb/milestones{/number}","notifications_url":"https://api.github.com/repos/greenplum-db/gpdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/greenplum-db/gpdb/labels{/name}","releases_url":"https://api.github.com/repos/greenplum-db/gpdb/releases{/id}","deployments_url":"https://api.github.com/repos/greenplum-db/gpdb/deployments","created_at":"2015-10-23T00:25:17Z","updated_at":"2016-06-09T19:22:52Z","pushed_at":"2016-06-09T22:28:15Z","git_url":"git://github.com/greenplum-db/gpdb.git","ssh_url":"git@github.com:greenplum-db/gpdb.git","clone_url":"https://github.com/greenplum-db/gpdb.git","svn_url":"https://github.com/greenplum-db/gpdb","homepage":"http://greenplum.org","size":155699,"stargazers_count":1465,"watchers_count":1465,"language":"PLpgSQL","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":392,"mirror_url":null,"open_issues_count":92,"forks":392,"open_issues":92,"watchers":1465,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825"},"html":{"href":"https://github.com/greenplum-db/gpdb/pull/825"},"issue":{"href":"https://api.github.com/repos/greenplum-db/gpdb/issues/825"},"comments":{"href":"https://api.github.com/repos/greenplum-db/gpdb/issues/825/comments"},"review_comments":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825/comments"},"review_comment":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/greenplum-db/gpdb/pulls/825/commits"},"statuses":{"href":"https://api.github.com/repos/greenplum-db/gpdb/statuses/c34959fa4b4d9a69b432b7d2c2097b245ae270fa"}}}}
{"actor":{"display_login":"foyzur"}}