{"action":"created","issue":{"url":"https://api.github.com/repos/greenplum-db/gpdb/issues/132","labels_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/132/labels{/name}","comments_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/132/comments","events_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/132/events","html_url":"https://github.com/greenplum-db/gpdb/pull/132","id":119775702,"number":132,"title":"Enable immediate response to SIGTERM when hanging at recv.","user":{"login":"eurekaka","id":6261973,"avatar_url":"https://avatars.githubusercontent.com/u/6261973?v=3","gravatar_id":"","url":"https://api.github.com/users/eurekaka","html_url":"https://github.com/eurekaka","followers_url":"https://api.github.com/users/eurekaka/followers","following_url":"https://api.github.com/users/eurekaka/following{/other_user}","gists_url":"https://api.github.com/users/eurekaka/gists{/gist_id}","starred_url":"https://api.github.com/users/eurekaka/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eurekaka/subscriptions","organizations_url":"https://api.github.com/users/eurekaka/orgs","repos_url":"https://api.github.com/users/eurekaka/repos","events_url":"https://api.github.com/users/eurekaka/events{/privacy}","received_events_url":"https://api.github.com/users/eurekaka/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2015-12-01T18:12:24Z","updated_at":"2015-12-03T17:20:23Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/greenplum-db/gpdb/pulls/132","html_url":"https://github.com/greenplum-db/gpdb/pull/132","diff_url":"https://github.com/greenplum-db/gpdb/pull/132.diff","patch_url":"https://github.com/greenplum-db/gpdb/pull/132.patch"},"body":"This addresses the problem that we cannot terminate QD by pg_terminate_backend()\r\nif someone issues 'COPY tbl FROM stdin' and then leaves forever."},"comment":{"url":"https://api.github.com/repos/greenplum-db/gpdb/issues/comments/161721458","html_url":"https://github.com/greenplum-db/gpdb/pull/132#issuecomment-161721458","issue_url":"https://api.github.com/repos/greenplum-db/gpdb/issues/132","id":161721458,"user":{"login":"hlinnaka","id":191602,"avatar_url":"https://avatars.githubusercontent.com/u/191602?v=3","gravatar_id":"","url":"https://api.github.com/users/hlinnaka","html_url":"https://github.com/hlinnaka","followers_url":"https://api.github.com/users/hlinnaka/followers","following_url":"https://api.github.com/users/hlinnaka/following{/other_user}","gists_url":"https://api.github.com/users/hlinnaka/gists{/gist_id}","starred_url":"https://api.github.com/users/hlinnaka/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hlinnaka/subscriptions","organizations_url":"https://api.github.com/users/hlinnaka/orgs","repos_url":"https://api.github.com/users/hlinnaka/repos","events_url":"https://api.github.com/users/hlinnaka/events{/privacy}","received_events_url":"https://api.github.com/users/hlinnaka/received_events","type":"User","site_admin":false},"created_at":"2015-12-03T17:20:23Z","updated_at":"2015-12-03T17:20:23Z","body":"On 03/12/15 19:12, Kenan Yao wrote:\n> Sure, I would go through the pgsql-hackers threads.\n> By the way, here is a snippet of function ProcessClientReadInterrupt from PostgreSQL master branch, which is called in secure_read:\n> ```\n> void\n> ProcessClientReadInterrupt(bool blocked)\n> {\n> \tint\t\t\tsave_errno = errno;\n>\n> \tif (DoingCommandRead)\n> \t{\n> \t\t/* Check for general interrupts that arrived while reading */\n> \t\tCHECK_FOR_INTERRUPTS();\n>\n> \t\t/* Process sinval catchup interrupts that happened while reading */\n> \t\tif (catchupInterruptPending)\n> \t\t\tProcessCatchupInterrupt();\n>\n> \t\t/* Process sinval catchup interrupts that happened while reading */\n> \t\tif (notifyInterruptPending)\n> \t\t\tProcessNotifyInterrupt();\n> \t}\n> \telse if (ProcDiePending && blocked)\n> \t{\n> \t\t/*\n> \t\t * We're dying. It's safe (and sane) to handle that now.\n> \t\t */\n> \t\tCHECK_FOR_INTERRUPTS();\n> \t}\n>\n> \terrno = save_errno;\n> }\n> ```\n> This means, even when DoingCommandRead is not true, it is still safe to die here in secure_read. The difference between this and my fix is that we are going to probably interrupt the recv() call in the middle, and I think that should be fine. Anyway, I would check the pgsql-hackers threads to see if there is anything missed.\n\nNote that there are tons more signal handling changes in PostgreSQL \nmaster. ImmediateInterruptOK doesn't exist anymore, the client socket is \nalways used in non-blocking mode, and sleeping to wait for client input \nis no longer done inside OpenSSL code. You can't draw much conclusions \nabout the safety of the proposed PR by looking at the PostgreSQL master \ncode. But do read the pgsql-hackers discussions that lead to those changes.\n\n>> Another thing is, for the race condition example you mentioned,\n>> Only then decide if this really is safe, and if the race conditions\n>> this still has are acceptable. (For example, pg_terminate_backend()\n>> would still not kill a backend if the signal arrives just before\n>> you enter the recv() call.)\n>\n> I think it can still kill the backend in this case, since we have set\n> ImmediateInterruptOK to be true before entering recv() call, and in\n> the handler function die() it would call ProcessInterrupts() to exit.\n\nAh yeah, you're right.\n\n- Heikki\n\n"}}