{"action":"created","issue":{"url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1147","repository_url":"https://api.github.com/repos/greenplum-db/gpdb","labels_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1147/labels{/name}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1147/comments","events_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1147/events","html_url":"https://github.com/greenplum-db/gpdb/pull/1147","id":178687590,"number":1147,"title":"130306049 segspace","user":{"login":"xinzweb","id":723898,"avatar_url":"https://avatars.githubusercontent.com/u/723898?v=3","gravatar_id":"","url":"https://api.github.com/users/xinzweb","html_url":"https://github.com/xinzweb","followers_url":"https://api.github.com/users/xinzweb/followers","following_url":"https://api.github.com/users/xinzweb/following{/other_user}","gists_url":"https://api.github.com/users/xinzweb/gists{/gist_id}","starred_url":"https://api.github.com/users/xinzweb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/xinzweb/subscriptions","organizations_url":"https://api.github.com/users/xinzweb/orgs","repos_url":"https://api.github.com/users/xinzweb/repos","events_url":"https://api.github.com/users/xinzweb/events{/privacy}","received_events_url":"https://api.github.com/users/xinzweb/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":8,"created_at":"2016-09-22T18:37:22Z","updated_at":"2016-09-29T23:09:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/1147","html_url":"https://github.com/greenplum-db/gpdb/pull/1147","diff_url":"https://github.com/greenplum-db/gpdb/pull/1147.diff","patch_url":"https://github.com/greenplum-db/gpdb/pull/1147.patch"},"body":"GPDB currently has 2 APIs of creating work files\r\n\r\n1. Using workfile\\_mgr :\r\n\r\n  1. This uses a workfile set to contain all the related workfiles.\r\n  2. All workfiles are created under the same directory. As a result, when it is time to delete the workfiles, we can simply delete the directory.\r\n  3. There is no mechanism to control the exact name of the workfile created. All workfiles are referenced using an ID. `\"%s/spillfile\\_f%u\", work\\_set-\\>path, file\\_no);`\r\n  4. Deletion of workiles is automatically handled in case of an exception, as all the files are contained in one directory, thus no counter/files are leaked.\r\n  \r\n2. Using ExecWorkfile :\r\n\r\n  1. There is a Create/Open/Read/Write/Flush/Close API.\r\n  2. This is the lower-level API also used by workfile\\_mgr to manage the workfiles directly.\r\n  3. Each operator is resposible to correctly close the workfile it creates.\r\n\r\nWhen we write to a workfile, we reserve and add the number of bytes to used\\_segspace, and add that amount to `workfile->size`. Then, we call `workfile_update_in_progress_size`. If the workfile has an owning workset, we update the workset's `in_progress_size` counter. However, if it has no such workset, we completely ignore the count - and this is what ultimately leaks the `used_segspace` counter.\r\n\r\nThere are few places in GPDB that use the lower level API to spill:\r\n1. `LogicalTapeSet`\r\n2. `Tuplesortstate`\r\n3. `ntuplestore_create_spill_files` \r\n4. `AppendOnlyVisimapDelete`\r\n5. `tuplesort_begin_heap_file_readerwriter_mk`\r\n\r\nIn normal execution, this does not leak because `ExecutorEnd` is called correctly - and that deletes the files, and updates the counters correctly. However when an exception occurs during execution, `ExecutorEnd` is only called for `SELECT` queries. For `UPDATE`, `INSERT`, `DELETE` queries- we don't call `ExecutorEnd` and thus leak files and the counter.\r\n\r\nThis is a quick fix to account for bytes written in workfiles that are not part of any workfile set - that is those created using the lower level API. Hence when we abort or error out, we subtract this amount from the `used_segspace` counter. \r\n\r\nIn the long term, having two APIs is cumbersome and error-prone, and we have created a story to fix it."},"comment":{"url":"https://api.github.com/repos/greenplum-db/gpdb/issues/comments/250617672","html_url":"https://github.com/greenplum-db/gpdb/pull/1147#issuecomment-250617672","issue_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/1147","id":250617672,"user":{"login":"gpdbdreddbot","id":15618396,"avatar_url":"https://avatars.githubusercontent.com/u/15618396?v=3","gravatar_id":"","url":"https://api.github.com/users/gpdbdreddbot","html_url":"https://github.com/gpdbdreddbot","followers_url":"https://api.github.com/users/gpdbdreddbot/followers","following_url":"https://api.github.com/users/gpdbdreddbot/following{/other_user}","gists_url":"https://api.github.com/users/gpdbdreddbot/gists{/gist_id}","starred_url":"https://api.github.com/users/gpdbdreddbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gpdbdreddbot/subscriptions","organizations_url":"https://api.github.com/users/gpdbdreddbot/orgs","repos_url":"https://api.github.com/users/gpdbdreddbot/repos","events_url":"https://api.github.com/users/gpdbdreddbot/events{/privacy}","received_events_url":"https://api.github.com/users/gpdbdreddbot/received_events","type":"User","site_admin":false},"created_at":"2016-09-29T23:09:24Z","updated_at":"2016-09-29T23:09:24Z","body":"Hello xinzweb!\n\nThanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.\n"}}
{"actor":{"display_login":"gpdbdreddbot"}}