{"action":"created","comment":{"url":"https://api.github.com/repos/Kong/kong/pulls/comments/241473198","pull_request_review_id":184754877,"id":241473198,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDI0MTQ3MzE5OA==","diff_hunk":"@@ -201,32 +227,13 @@ do\n       return nil, \"error attempting to acquire build_router lock: \" .. err\n     end\n \n-    -- lock acquired but we might not need to rebuild the router (if we\n-    -- were not the first request in this process to enter this code path)\n-    -- check again and rebuild if necessary\n-\n-    version, err = cache:get(\"router:version\", CACHE_ROUTER_OPTS, utils.uuid)\n-    if err then\n-      log(ngx.CRIT, \"could not ensure router is up to date: \", err)\n-      return nil, err\n-    end\n-\n-    if version == router_version then\n-      return router\n-    end\n-\n-    -- router needs to be rebuilt in this worker\n-    log(DEBUG, \"rebuilding router\")\n-\n-    local ok, err = build_router(singletons.db, version)\n+    local ok, router, err = pcall(fetch_router)","path":"kong/runloop/handler.lua","position":89,"original_position":89,"commit_id":"f7985c71f6e1e9fa9f780fc0058b8d58b64caa27","original_commit_id":"f7985c71f6e1e9fa9f780fc0058b8d58b64caa27","user":{"login":"hishamhm","id":245621,"node_id":"MDQ6VXNlcjI0NTYyMQ==","avatar_url":"https://avatars3.githubusercontent.com/u/245621?v=4","gravatar_id":"","url":"https://api.github.com/users/hishamhm","html_url":"https://github.com/hishamhm","followers_url":"https://api.github.com/users/hishamhm/followers","following_url":"https://api.github.com/users/hishamhm/following{/other_user}","gists_url":"https://api.github.com/users/hishamhm/gists{/gist_id}","starred_url":"https://api.github.com/users/hishamhm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hishamhm/subscriptions","organizations_url":"https://api.github.com/users/hishamhm/orgs","repos_url":"https://api.github.com/users/hishamhm/repos","events_url":"https://api.github.com/users/hishamhm/events{/privacy}","received_events_url":"https://api.github.com/users/hishamhm/received_events","type":"User","site_admin":false},"body":"I'm not expecting runtime errors here, I think @bungle was just being defensive (I didn't have the `pcall` in my version of the patch). From a readability perspective, I think \"just-in-case\" pcalls tend to throw off the reader a little (e.g. implying that there are expected runtime `errors` in the code path). But given that an `error`-causing bug in this sequence would lock down the worker, I don't mind keeping it.","created_at":"2018-12-13T16:36:42Z","updated_at":"2018-12-13T16:36:42Z","html_url":"https://github.com/Kong/kong/pull/4084#discussion_r241473198","pull_request_url":"https://api.github.com/repos/Kong/kong/pulls/4084","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/Kong/kong/pulls/comments/241473198"},"html":{"href":"https://github.com/Kong/kong/pull/4084#discussion_r241473198"},"pull_request":{"href":"https://api.github.com/repos/Kong/kong/pulls/4084"}},"in_reply_to_id":241466718},"pull_request":{"url":"https://api.github.com/repos/Kong/kong/pulls/4084","id":238387991,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM4Mzg3OTkx","html_url":"https://github.com/Kong/kong/pull/4084","diff_url":"https://github.com/Kong/kong/pull/4084.diff","patch_url":"https://github.com/Kong/kong/pull/4084.patch","issue_url":"https://api.github.com/repos/Kong/kong/issues/4084","number":4084,"state":"open","locked":false,"title":"fix(core) ensures that router semaphore is always released","user":{"login":"bungle","id":983257,"node_id":"MDQ6VXNlcjk4MzI1Nw==","avatar_url":"https://avatars0.githubusercontent.com/u/983257?v=4","gravatar_id":"","url":"https://api.github.com/users/bungle","html_url":"https://github.com/bungle","followers_url":"https://api.github.com/users/bungle/followers","following_url":"https://api.github.com/users/bungle/following{/other_user}","gists_url":"https://api.github.com/users/bungle/gists{/gist_id}","starred_url":"https://api.github.com/users/bungle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bungle/subscriptions","organizations_url":"https://api.github.com/users/bungle/orgs","repos_url":"https://api.github.com/users/bungle/repos","events_url":"https://api.github.com/users/bungle/events{/privacy}","received_events_url":"https://api.github.com/users/bungle/received_events","type":"User","site_admin":false},"body":"### Summary\r\n\r\nThis PR ensures that acquired `semaphore` is always released, no matter what (even on runtime errors) when building a router. This only affects routes and services router as the old api router is good enough (it doesnt's handle runtime-errors, but other than that it is solid, and it will be soon gone anyway — let me know if you want me to look into it too).\r\n\r\nThe issue related to this was reported by @jeremyjpj0916 on #4055.\r\n\r\nThere is still another issue with new router rebuilding. The api router uses `find_all`, while new router uses `each`(makes multiple paged requests instead) and separately fetches service by `id`. The problem is when there are new routes added or deleted during router rebuilding, which can lead to some routes missed. I guess there is also a slight possiblity to feach route and when trying to fetch service, both are already deleted meanwhile. This PR doesn't fix that totally, as that only affects with bad luck a couple of requests. When changes are done, the right router will be built and cached. All that said, I did make this issue a bit less so, as I changed `routes:each` to use the maximum `1000` (I think it wouldn't hurt if we used something like `10000` but then we need to lift the maximum) as page size instead of the default `100`. This can also make rebuilding a bit faster with huge number of routes, but not sure if it is noticeable.\r\n\r\n@jeremyjpj0916 can you check if this makes in your testing this issue go away, or very very rare at least?","created_at":"2018-12-13T13:23:03Z","updated_at":"2018-12-13T16:36:42Z","closed_at":null,"merged_at":null,"merge_commit_sha":"9f1ff25804f1c52a7f0d8bd58d2f0a17e4ecb386","assignee":null,"assignees":[],"requested_reviewers":[{"login":"hishamhm","id":245621,"node_id":"MDQ6VXNlcjI0NTYyMQ==","avatar_url":"https://avatars3.githubusercontent.com/u/245621?v=4","gravatar_id":"","url":"https://api.github.com/users/hishamhm","html_url":"https://github.com/hishamhm","followers_url":"https://api.github.com/users/hishamhm/followers","following_url":"https://api.github.com/users/hishamhm/following{/other_user}","gists_url":"https://api.github.com/users/hishamhm/gists{/gist_id}","starred_url":"https://api.github.com/users/hishamhm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hishamhm/subscriptions","organizations_url":"https://api.github.com/users/hishamhm/orgs","repos_url":"https://api.github.com/users/hishamhm/repos","events_url":"https://api.github.com/users/hishamhm/events{/privacy}","received_events_url":"https://api.github.com/users/hishamhm/received_events","type":"User","site_admin":false},{"login":"p0pr0ck5","id":4625425,"node_id":"MDQ6VXNlcjQ2MjU0MjU=","avatar_url":"https://avatars0.githubusercontent.com/u/4625425?v=4","gravatar_id":"","url":"https://api.github.com/users/p0pr0ck5","html_url":"https://github.com/p0pr0ck5","followers_url":"https://api.github.com/users/p0pr0ck5/followers","following_url":"https://api.github.com/users/p0pr0ck5/following{/other_user}","gists_url":"https://api.github.com/users/p0pr0ck5/gists{/gist_id}","starred_url":"https://api.github.com/users/p0pr0ck5/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/p0pr0ck5/subscriptions","organizations_url":"https://api.github.com/users/p0pr0ck5/orgs","repos_url":"https://api.github.com/users/p0pr0ck5/repos","events_url":"https://api.github.com/users/p0pr0ck5/events{/privacy}","received_events_url":"https://api.github.com/users/p0pr0ck5/received_events","type":"User","site_admin":false}],"requested_teams":[],"labels":[{"id":272670282,"node_id":"MDU6TGFiZWwyNzI2NzAyODI=","url":"https://api.github.com/repos/Kong/kong/labels/pr/do%20not%20merge","name":"pr/do not merge","color":"dd275e","default":false},{"id":219207429,"node_id":"MDU6TGFiZWwyMTkyMDc0Mjk=","url":"https://api.github.com/repos/Kong/kong/labels/pr/please%20review","name":"pr/please review","color":"8c48ea","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/Kong/kong/pulls/4084/commits","review_comments_url":"https://api.github.com/repos/Kong/kong/pulls/4084/comments","review_comment_url":"https://api.github.com/repos/Kong/kong/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Kong/kong/issues/4084/comments","statuses_url":"https://api.github.com/repos/Kong/kong/statuses/f7985c71f6e1e9fa9f780fc0058b8d58b64caa27","head":{"label":"Kong:fix/semaphore-release","ref":"fix/semaphore-release","sha":"f7985c71f6e1e9fa9f780fc0058b8d58b64caa27","user":{"login":"Kong","id":962416,"node_id":"MDEyOk9yZ2FuaXphdGlvbjk2MjQxNg==","avatar_url":"https://avatars0.githubusercontent.com/u/962416?v=4","gravatar_id":"","url":"https://api.github.com/users/Kong","html_url":"https://github.com/Kong","followers_url":"https://api.github.com/users/Kong/followers","following_url":"https://api.github.com/users/Kong/following{/other_user}","gists_url":"https://api.github.com/users/Kong/gists{/gist_id}","starred_url":"https://api.github.com/users/Kong/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Kong/subscriptions","organizations_url":"https://api.github.com/users/Kong/orgs","repos_url":"https://api.github.com/users/Kong/repos","events_url":"https://api.github.com/users/Kong/events{/privacy}","received_events_url":"https://api.github.com/users/Kong/received_events","type":"Organization","site_admin":false},"repo":{"id":26783295,"node_id":"MDEwOlJlcG9zaXRvcnkyNjc4MzI5NQ==","name":"kong","full_name":"Kong/kong","private":false,"owner":{"login":"Kong","id":962416,"node_id":"MDEyOk9yZ2FuaXphdGlvbjk2MjQxNg==","avatar_url":"https://avatars0.githubusercontent.com/u/962416?v=4","gravatar_id":"","url":"https://api.github.com/users/Kong","html_url":"https://github.com/Kong","followers_url":"https://api.github.com/users/Kong/followers","following_url":"https://api.github.com/users/Kong/following{/other_user}","gists_url":"https://api.github.com/users/Kong/gists{/gist_id}","starred_url":"https://api.github.com/users/Kong/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Kong/subscriptions","organizations_url":"https://api.github.com/users/Kong/orgs","repos_url":"https://api.github.com/users/Kong/repos","events_url":"https://api.github.com/users/Kong/events{/privacy}","received_events_url":"https://api.github.com/users/Kong/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/Kong/kong","description":":gorilla: The Cloud-Native API Gateway","fork":false,"url":"https://api.github.com/repos/Kong/kong","forks_url":"https://api.github.com/repos/Kong/kong/forks","keys_url":"https://api.github.com/repos/Kong/kong/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Kong/kong/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Kong/kong/teams","hooks_url":"https://api.github.com/repos/Kong/kong/hooks","issue_events_url":"https://api.github.com/repos/Kong/kong/issues/events{/number}","events_url":"https://api.github.com/repos/Kong/kong/events","assignees_url":"https://api.github.com/repos/Kong/kong/assignees{/user}","branches_url":"https://api.github.com/repos/Kong/kong/branches{/branch}","tags_url":"https://api.github.com/repos/Kong/kong/tags","blobs_url":"https://api.github.com/repos/Kong/kong/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Kong/kong/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Kong/kong/git/refs{/sha}","trees_url":"https://api.github.com/repos/Kong/kong/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Kong/kong/statuses/{sha}","languages_url":"https://api.github.com/repos/Kong/kong/languages","stargazers_url":"https://api.github.com/repos/Kong/kong/stargazers","contributors_url":"https://api.github.com/repos/Kong/kong/contributors","subscribers_url":"https://api.github.com/repos/Kong/kong/subscribers","subscription_url":"https://api.github.com/repos/Kong/kong/subscription","commits_url":"https://api.github.com/repos/Kong/kong/commits{/sha}","git_commits_url":"https://api.github.com/repos/Kong/kong/git/commits{/sha}","comments_url":"https://api.github.com/repos/Kong/kong/comments{/number}","issue_comment_url":"https://api.github.com/repos/Kong/kong/issues/comments{/number}","contents_url":"https://api.github.com/repos/Kong/kong/contents/{+path}","compare_url":"https://api.github.com/repos/Kong/kong/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Kong/kong/merges","archive_url":"https://api.github.com/repos/Kong/kong/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Kong/kong/downloads","issues_url":"https://api.github.com/repos/Kong/kong/issues{/number}","pulls_url":"https://api.github.com/repos/Kong/kong/pulls{/number}","milestones_url":"https://api.github.com/repos/Kong/kong/milestones{/number}","notifications_url":"https://api.github.com/repos/Kong/kong/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Kong/kong/labels{/name}","releases_url":"https://api.github.com/repos/Kong/kong/releases{/id}","deployments_url":"https://api.github.com/repos/Kong/kong/deployments","created_at":"2014-11-17T23:56:08Z","updated_at":"2018-12-13T14:46:45Z","pushed_at":"2018-12-13T16:28:22Z","git_url":"git://github.com/Kong/kong.git","ssh_url":"git@github.com:Kong/kong.git","clone_url":"https://github.com/Kong/kong.git","svn_url":"https://github.com/Kong/kong","homepage":"https://konghq.com/install","size":11065,"stargazers_count":19125,"watchers_count":19125,"language":"Lua","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2300,"mirror_url":null,"archived":false,"open_issues_count":323,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":2300,"open_issues":323,"watchers":19125,"default_branch":"master"}},"base":{"label":"Kong:next","ref":"next","sha":"4ef9df5e09eb1a771b8f32e45de2cd39ba807a2a","user":{"login":"Kong","id":962416,"node_id":"MDEyOk9yZ2FuaXphdGlvbjk2MjQxNg==","avatar_url":"https://avatars0.githubusercontent.com/u/962416?v=4","gravatar_id":"","url":"https://api.github.com/users/Kong","html_url":"https://github.com/Kong","followers_url":"https://api.github.com/users/Kong/followers","following_url":"https://api.github.com/users/Kong/following{/other_user}","gists_url":"https://api.github.com/users/Kong/gists{/gist_id}","starred_url":"https://api.github.com/users/Kong/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Kong/subscriptions","organizations_url":"https://api.github.com/users/Kong/orgs","repos_url":"https://api.github.com/users/Kong/repos","events_url":"https://api.github.com/users/Kong/events{/privacy}","received_events_url":"https://api.github.com/users/Kong/received_events","type":"Organization","site_admin":false},"repo":{"id":26783295,"node_id":"MDEwOlJlcG9zaXRvcnkyNjc4MzI5NQ==","name":"kong","full_name":"Kong/kong","private":false,"owner":{"login":"Kong","id":962416,"node_id":"MDEyOk9yZ2FuaXphdGlvbjk2MjQxNg==","avatar_url":"https://avatars0.githubusercontent.com/u/962416?v=4","gravatar_id":"","url":"https://api.github.com/users/Kong","html_url":"https://github.com/Kong","followers_url":"https://api.github.com/users/Kong/followers","following_url":"https://api.github.com/users/Kong/following{/other_user}","gists_url":"https://api.github.com/users/Kong/gists{/gist_id}","starred_url":"https://api.github.com/users/Kong/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Kong/subscriptions","organizations_url":"https://api.github.com/users/Kong/orgs","repos_url":"https://api.github.com/users/Kong/repos","events_url":"https://api.github.com/users/Kong/events{/privacy}","received_events_url":"https://api.github.com/users/Kong/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/Kong/kong","description":":gorilla: The Cloud-Native API Gateway","fork":false,"url":"https://api.github.com/repos/Kong/kong","forks_url":"https://api.github.com/repos/Kong/kong/forks","keys_url":"https://api.github.com/repos/Kong/kong/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Kong/kong/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Kong/kong/teams","hooks_url":"https://api.github.com/repos/Kong/kong/hooks","issue_events_url":"https://api.github.com/repos/Kong/kong/issues/events{/number}","events_url":"https://api.github.com/repos/Kong/kong/events","assignees_url":"https://api.github.com/repos/Kong/kong/assignees{/user}","branches_url":"https://api.github.com/repos/Kong/kong/branches{/branch}","tags_url":"https://api.github.com/repos/Kong/kong/tags","blobs_url":"https://api.github.com/repos/Kong/kong/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Kong/kong/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Kong/kong/git/refs{/sha}","trees_url":"https://api.github.com/repos/Kong/kong/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Kong/kong/statuses/{sha}","languages_url":"https://api.github.com/repos/Kong/kong/languages","stargazers_url":"https://api.github.com/repos/Kong/kong/stargazers","contributors_url":"https://api.github.com/repos/Kong/kong/contributors","subscribers_url":"https://api.github.com/repos/Kong/kong/subscribers","subscription_url":"https://api.github.com/repos/Kong/kong/subscription","commits_url":"https://api.github.com/repos/Kong/kong/commits{/sha}","git_commits_url":"https://api.github.com/repos/Kong/kong/git/commits{/sha}","comments_url":"https://api.github.com/repos/Kong/kong/comments{/number}","issue_comment_url":"https://api.github.com/repos/Kong/kong/issues/comments{/number}","contents_url":"https://api.github.com/repos/Kong/kong/contents/{+path}","compare_url":"https://api.github.com/repos/Kong/kong/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Kong/kong/merges","archive_url":"https://api.github.com/repos/Kong/kong/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Kong/kong/downloads","issues_url":"https://api.github.com/repos/Kong/kong/issues{/number}","pulls_url":"https://api.github.com/repos/Kong/kong/pulls{/number}","milestones_url":"https://api.github.com/repos/Kong/kong/milestones{/number}","notifications_url":"https://api.github.com/repos/Kong/kong/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Kong/kong/labels{/name}","releases_url":"https://api.github.com/repos/Kong/kong/releases{/id}","deployments_url":"https://api.github.com/repos/Kong/kong/deployments","created_at":"2014-11-17T23:56:08Z","updated_at":"2018-12-13T14:46:45Z","pushed_at":"2018-12-13T16:28:22Z","git_url":"git://github.com/Kong/kong.git","ssh_url":"git@github.com:Kong/kong.git","clone_url":"https://github.com/Kong/kong.git","svn_url":"https://github.com/Kong/kong","homepage":"https://konghq.com/install","size":11065,"stargazers_count":19125,"watchers_count":19125,"language":"Lua","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2300,"mirror_url":null,"archived":false,"open_issues_count":323,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":2300,"open_issues":323,"watchers":19125,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Kong/kong/pulls/4084"},"html":{"href":"https://github.com/Kong/kong/pull/4084"},"issue":{"href":"https://api.github.com/repos/Kong/kong/issues/4084"},"comments":{"href":"https://api.github.com/repos/Kong/kong/issues/4084/comments"},"review_comments":{"href":"https://api.github.com/repos/Kong/kong/pulls/4084/comments"},"review_comment":{"href":"https://api.github.com/repos/Kong/kong/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Kong/kong/pulls/4084/commits"},"statuses":{"href":"https://api.github.com/repos/Kong/kong/statuses/f7985c71f6e1e9fa9f780fc0058b8d58b64caa27"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"hishamhm"}}