{"action":"created","comment":{"url":"https://api.github.com/repos/ceph/ceph/pulls/comments/178644466","pull_request_review_id":108735460,"id":178644466,"diff_hunk":"@@ -445,7 +445,8 @@ class PG : public DoutPrefixProvider {\n   void get_pg_stats(std::function<void(const pg_stat_t&, epoch_t lec)> f);\n   void with_heartbeat_peers(std::function<void(int)> f);\n \n-  virtual void shutdown() = 0;\n+  void shutdown();","path":"src/osd/PG.h","position":38,"original_position":5,"commit_id":"40b2163ca6ae6ad9e320c023a4d2395d7c87bb24","original_commit_id":"83b1083c713ac5e0ee56f5ab2afe34bafb058854","user":{"login":"gregsfortytwo","id":908479,"avatar_url":"https://avatars2.githubusercontent.com/u/908479?v=4","gravatar_id":"","url":"https://api.github.com/users/gregsfortytwo","html_url":"https://github.com/gregsfortytwo","followers_url":"https://api.github.com/users/gregsfortytwo/followers","following_url":"https://api.github.com/users/gregsfortytwo/following{/other_user}","gists_url":"https://api.github.com/users/gregsfortytwo/gists{/gist_id}","starred_url":"https://api.github.com/users/gregsfortytwo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gregsfortytwo/subscriptions","organizations_url":"https://api.github.com/users/gregsfortytwo/orgs","repos_url":"https://api.github.com/users/gregsfortytwo/repos","events_url":"https://api.github.com/users/gregsfortytwo/events{/privacy}","received_events_url":"https://api.github.com/users/gregsfortytwo/received_events","type":"User","site_admin":false},"body":"Right, but we still have that virtual ```on_shutdown()``` function that gets invoked at basically the same time. The patch making the switch saves neither lines of code nor a vtable lookup, and I don't see what motivated it","created_at":"2018-04-02T20:41:17Z","updated_at":"2018-04-02T20:41:17Z","html_url":"https://github.com/ceph/ceph/pull/19973#discussion_r178644466","pull_request_url":"https://api.github.com/repos/ceph/ceph/pulls/19973","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/ceph/ceph/pulls/comments/178644466"},"html":{"href":"https://github.com/ceph/ceph/pull/19973#discussion_r178644466"},"pull_request":{"href":"https://api.github.com/repos/ceph/ceph/pulls/19973"}},"in_reply_to_id":178415789},"pull_request":{"url":"https://api.github.com/repos/ceph/ceph/pulls/19973","id":163299005,"html_url":"https://github.com/ceph/ceph/pull/19973","diff_url":"https://github.com/ceph/ceph/pull/19973.diff","patch_url":"https://github.com/ceph/ceph/pull/19973.patch","issue_url":"https://api.github.com/repos/ceph/ceph/issues/19973","number":19973,"state":"open","locked":false,"title":"osd: fast dispatch of peering events and pg_map + osd sharded wq refactor","user":{"login":"liewegas","id":433031,"avatar_url":"https://avatars1.githubusercontent.com/u/433031?v=4","gravatar_id":"","url":"https://api.github.com/users/liewegas","html_url":"https://github.com/liewegas","followers_url":"https://api.github.com/users/liewegas/followers","following_url":"https://api.github.com/users/liewegas/following{/other_user}","gists_url":"https://api.github.com/users/liewegas/gists{/gist_id}","starred_url":"https://api.github.com/users/liewegas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liewegas/subscriptions","organizations_url":"https://api.github.com/users/liewegas/orgs","repos_url":"https://api.github.com/users/liewegas/repos","events_url":"https://api.github.com/users/liewegas/events{/privacy}","received_events_url":"https://api.github.com/users/liewegas/received_events","type":"User","site_admin":false},"body":"- put everything in the fast dispatch path *except* for osd map processing and legacy pg\r\ncreate messages\r\n- move most of the guts of ShardedOpWQ into a top-level OSDShard\r\n- kill the global pg_map and instead rely on the pg_slots in each OSDShard\r\n- several cleansup along the way\r\n- some bug fixes that seem related to pg removal but didn't surface on master (since this will merge before mimic not bothering to try to pull them out)\r\n\r\nTODO\r\n- [x] write doc for op and peering event ordering constraints\r\n\r\nReview guide: This is a long patch series that represents a transition between several different approaches, eventually settling on a more siginificant rewrite of the sharded wq than I had originally planned.  Sorry!  That means that the intermediate solutions aren't worth reviewing carefully for correctness, but are still useful to see separately to understand the evolution of the code.\r\n\r\nThis series goes roughly like this:\r\n- *fast dispatch of peering events*.  The first several commits, up through \"osd: fast dispatch peering events\" (parts 1 and 2) simplify the OSD.cc code so that peering messages translate directly into one or more events that are fed into the normal sharded work queue.  This simplifies the number of paths through the code, making it easier to reason about.\r\n- Several patches change the pg creation behavior.  The overall takeaway here is that there is an event property that determines whether the event that trigger a pg creation.  Several events do this, including notify, log, and of course the create message from the mon.  We have a new MOSDPGCreate2 message that specifies those creations in terms of spg_t instead of pg_t.  Unfortunatley we have to keep around the old creation path (hadnle_pg_create) to deal with old monitors.  That will eventaully go away, butwe have to have a couple releases for that.\r\n- 'osd: prime pg_slots for to-be-split children' is notable for introducing a pg_slot-based way to deal with splits: instnatiate the slot for the child pgs and mark it so that we queue up events but don't process them yet.  This will eventually avoids special check for \"is this pg splitting?\" that have to look things up, although those changes come a bit later.\r\n- Move scrub requests into teh work queue.\r\n- Kill the disk_tp!  Yay!  Now we only have two work queues: the big one, and one for commands (asok or tell).  The latter is totally unrelated to PGs so for now let's leave it separate.\r\n- Here's where things get messy: several patches adjust (and adjust again) how the PG waiting works.  This isn't worth looking at in detail; instead, look at the final result and the comments in OSD.h above _process that describe the rules around queueing events, peering vs op events, and how the requeuing behaves.\r\n- Several patches around \"osd: store ec profile with final pool\" resolve an issue with the idea that a PG survives until its data is deleted, even if the pool is gone.  That means we have reinstantiate the PG on an OSD restart even if the pool info is missing from teh OSDmap.  To that end, we stash info about deleted pools... namely the pg_num and the erasure-code-profile.\r\n- 'osd: do not release recovery_ops_reserved on requeue' is notable because the old behavior made no sense to me, so i \"fixed\" it.. but that old behavior is still present (I think) in luminous.  Please see if I'm missing something!\r\n- lots of the lookup methos in OSD.h are simplified here, and even further later on when the number of ways we look up pgs continually shrinks (yay!).\r\n- 'osd: kill pg_map' is the next big shift: we remove pg_map, and rely entirely on each OSDShard's pg_slots table.  This simplifies everything because we have less stuff to keep consistent in memory and one less mutex to deal with.  It's a huge patch, and a lot of the code it touches gets changed later, so don't worry too much about correctness yet.\r\n- 'restructure consume_map' is the next big one.  The concept is that the map comes into handle_osd_map, is written to disk,a nd gets to consume_map (same as before).  At this point we push it down into each shard.  It's at this point that we make sure the shard's OSDMapRef and the pg_slots are in sync in terms of priming splits: once the shard's ref is updated we know that any resulting split is already primed.  Later, the advance_pg() bit that pulls PGs up to the shard map can thus be sure that any splits that result then are primed and the children can be installed as soon as they're ready.  The old split tracking machinery (lock and a bunch of random maps) is cleaned out, yay!\r\n- _attach_pg and _detach_pg unify the pg linkage to pg_slots in one place.  Also we now have a PG pointer back to its shard *and* its pg_slot.\r\n- pg_slots switch to unique_ptr... they're getting big so we don't want them to get copied around on hash inserts etc.  (Also, we want the pg backpointer!)\r\n- The pg_epoch tracking (to figure out the min epoch for all pgs) is moved to be per-shard.  Old machinery is removed.\r\n- RecoveryCtx completions handling is simplified (attach directly to txn, yay)\r\n- The pg counts are adjusted on PG removal.  This is good enough for now, I think, but I'm definitely looking for ways to improve this later.\r\n- 'osd: kill broken _process optimization; simplify null pg flow' is the last biggish rewrite.  The all-important _process flow is simplified with (I think) all remnants of complexity from the pg_map days removed.  The checks that are needed after dropping and retaking the shard lock are also grouped and only applied when the pg is non-null; if it's null we skip it all.\r\n- The remaining patches are small fixups.\r\n\r\nAt the end, I would review the _process() code itself along with the description of ordering in OSD.h.  This is the heart of it!","created_at":"2018-01-16T22:03:46Z","updated_at":"2018-04-02T20:41:17Z","closed_at":null,"merged_at":null,"merge_commit_sha":"23c68fc79916c3f52716bbcc1a94866545531b28","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":140959062,"url":"https://api.github.com/repos/ceph/ceph/labels/cleanup","name":"cleanup","color":"009800","default":false},{"id":144329561,"url":"https://api.github.com/repos/ceph/ceph/labels/core","name":"core","color":"0041a2","default":false},{"id":141186344,"url":"https://api.github.com/repos/ceph/ceph/labels/documentation","name":"documentation","color":"fef2c0","default":false},{"id":290749046,"url":"https://api.github.com/repos/ceph/ceph/labels/wip-sage2-testing","name":"wip-sage2-testing","color":"e11d21","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/ceph/ceph/pulls/19973/commits","review_comments_url":"https://api.github.com/repos/ceph/ceph/pulls/19973/comments","review_comment_url":"https://api.github.com/repos/ceph/ceph/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ceph/ceph/issues/19973/comments","statuses_url":"https://api.github.com/repos/ceph/ceph/statuses/40b2163ca6ae6ad9e320c023a4d2395d7c87bb24","head":{"label":"liewegas:wip-peering-fast-dispatch","ref":"wip-peering-fast-dispatch","sha":"40b2163ca6ae6ad9e320c023a4d2395d7c87bb24","user":{"login":"liewegas","id":433031,"avatar_url":"https://avatars1.githubusercontent.com/u/433031?v=4","gravatar_id":"","url":"https://api.github.com/users/liewegas","html_url":"https://github.com/liewegas","followers_url":"https://api.github.com/users/liewegas/followers","following_url":"https://api.github.com/users/liewegas/following{/other_user}","gists_url":"https://api.github.com/users/liewegas/gists{/gist_id}","starred_url":"https://api.github.com/users/liewegas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liewegas/subscriptions","organizations_url":"https://api.github.com/users/liewegas/orgs","repos_url":"https://api.github.com/users/liewegas/repos","events_url":"https://api.github.com/users/liewegas/events{/privacy}","received_events_url":"https://api.github.com/users/liewegas/received_events","type":"User","site_admin":false},"repo":{"id":2430918,"name":"ceph","full_name":"liewegas/ceph","owner":{"login":"liewegas","id":433031,"avatar_url":"https://avatars1.githubusercontent.com/u/433031?v=4","gravatar_id":"","url":"https://api.github.com/users/liewegas","html_url":"https://github.com/liewegas","followers_url":"https://api.github.com/users/liewegas/followers","following_url":"https://api.github.com/users/liewegas/following{/other_user}","gists_url":"https://api.github.com/users/liewegas/gists{/gist_id}","starred_url":"https://api.github.com/users/liewegas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/liewegas/subscriptions","organizations_url":"https://api.github.com/users/liewegas/orgs","repos_url":"https://api.github.com/users/liewegas/repos","events_url":"https://api.github.com/users/liewegas/events{/privacy}","received_events_url":"https://api.github.com/users/liewegas/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/liewegas/ceph","description":"Ceph distributed file system","fork":true,"url":"https://api.github.com/repos/liewegas/ceph","forks_url":"https://api.github.com/repos/liewegas/ceph/forks","keys_url":"https://api.github.com/repos/liewegas/ceph/keys{/key_id}","collaborators_url":"https://api.github.com/repos/liewegas/ceph/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/liewegas/ceph/teams","hooks_url":"https://api.github.com/repos/liewegas/ceph/hooks","issue_events_url":"https://api.github.com/repos/liewegas/ceph/issues/events{/number}","events_url":"https://api.github.com/repos/liewegas/ceph/events","assignees_url":"https://api.github.com/repos/liewegas/ceph/assignees{/user}","branches_url":"https://api.github.com/repos/liewegas/ceph/branches{/branch}","tags_url":"https://api.github.com/repos/liewegas/ceph/tags","blobs_url":"https://api.github.com/repos/liewegas/ceph/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/liewegas/ceph/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/liewegas/ceph/git/refs{/sha}","trees_url":"https://api.github.com/repos/liewegas/ceph/git/trees{/sha}","statuses_url":"https://api.github.com/repos/liewegas/ceph/statuses/{sha}","languages_url":"https://api.github.com/repos/liewegas/ceph/languages","stargazers_url":"https://api.github.com/repos/liewegas/ceph/stargazers","contributors_url":"https://api.github.com/repos/liewegas/ceph/contributors","subscribers_url":"https://api.github.com/repos/liewegas/ceph/subscribers","subscription_url":"https://api.github.com/repos/liewegas/ceph/subscription","commits_url":"https://api.github.com/repos/liewegas/ceph/commits{/sha}","git_commits_url":"https://api.github.com/repos/liewegas/ceph/git/commits{/sha}","comments_url":"https://api.github.com/repos/liewegas/ceph/comments{/number}","issue_comment_url":"https://api.github.com/repos/liewegas/ceph/issues/comments{/number}","contents_url":"https://api.github.com/repos/liewegas/ceph/contents/{+path}","compare_url":"https://api.github.com/repos/liewegas/ceph/compare/{base}...{head}","merges_url":"https://api.github.com/repos/liewegas/ceph/merges","archive_url":"https://api.github.com/repos/liewegas/ceph/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/liewegas/ceph/downloads","issues_url":"https://api.github.com/repos/liewegas/ceph/issues{/number}","pulls_url":"https://api.github.com/repos/liewegas/ceph/pulls{/number}","milestones_url":"https://api.github.com/repos/liewegas/ceph/milestones{/number}","notifications_url":"https://api.github.com/repos/liewegas/ceph/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/liewegas/ceph/labels{/name}","releases_url":"https://api.github.com/repos/liewegas/ceph/releases{/id}","deployments_url":"https://api.github.com/repos/liewegas/ceph/deployments","created_at":"2011-09-21T16:14:23Z","updated_at":"2017-08-21T03:09:50Z","pushed_at":"2018-04-02T20:33:51Z","git_url":"git://github.com/liewegas/ceph.git","ssh_url":"git@github.com:liewegas/ceph.git","clone_url":"https://github.com/liewegas/ceph.git","svn_url":"https://github.com/liewegas/ceph","homepage":"http://ceph.newdream.net/","size":275125,"stargazers_count":14,"watchers_count":14,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":6,"mirror_url":null,"archived":false,"open_issues_count":5,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":6,"open_issues":5,"watchers":14,"default_branch":"master"}},"base":{"label":"ceph:master","ref":"master","sha":"6aaa4a9bc4595cbad5c7d72a5d55ff8f691a03f9","user":{"login":"ceph","id":1015767,"avatar_url":"https://avatars3.githubusercontent.com/u/1015767?v=4","gravatar_id":"","url":"https://api.github.com/users/ceph","html_url":"https://github.com/ceph","followers_url":"https://api.github.com/users/ceph/followers","following_url":"https://api.github.com/users/ceph/following{/other_user}","gists_url":"https://api.github.com/users/ceph/gists{/gist_id}","starred_url":"https://api.github.com/users/ceph/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ceph/subscriptions","organizations_url":"https://api.github.com/users/ceph/orgs","repos_url":"https://api.github.com/users/ceph/repos","events_url":"https://api.github.com/users/ceph/events{/privacy}","received_events_url":"https://api.github.com/users/ceph/received_events","type":"Organization","site_admin":false},"repo":{"id":2310495,"name":"ceph","full_name":"ceph/ceph","owner":{"login":"ceph","id":1015767,"avatar_url":"https://avatars3.githubusercontent.com/u/1015767?v=4","gravatar_id":"","url":"https://api.github.com/users/ceph","html_url":"https://github.com/ceph","followers_url":"https://api.github.com/users/ceph/followers","following_url":"https://api.github.com/users/ceph/following{/other_user}","gists_url":"https://api.github.com/users/ceph/gists{/gist_id}","starred_url":"https://api.github.com/users/ceph/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ceph/subscriptions","organizations_url":"https://api.github.com/users/ceph/orgs","repos_url":"https://api.github.com/users/ceph/repos","events_url":"https://api.github.com/users/ceph/events{/privacy}","received_events_url":"https://api.github.com/users/ceph/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ceph/ceph","description":"Ceph is a distributed object, block, and file storage platform ","fork":false,"url":"https://api.github.com/repos/ceph/ceph","forks_url":"https://api.github.com/repos/ceph/ceph/forks","keys_url":"https://api.github.com/repos/ceph/ceph/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ceph/ceph/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ceph/ceph/teams","hooks_url":"https://api.github.com/repos/ceph/ceph/hooks","issue_events_url":"https://api.github.com/repos/ceph/ceph/issues/events{/number}","events_url":"https://api.github.com/repos/ceph/ceph/events","assignees_url":"https://api.github.com/repos/ceph/ceph/assignees{/user}","branches_url":"https://api.github.com/repos/ceph/ceph/branches{/branch}","tags_url":"https://api.github.com/repos/ceph/ceph/tags","blobs_url":"https://api.github.com/repos/ceph/ceph/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ceph/ceph/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ceph/ceph/git/refs{/sha}","trees_url":"https://api.github.com/repos/ceph/ceph/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ceph/ceph/statuses/{sha}","languages_url":"https://api.github.com/repos/ceph/ceph/languages","stargazers_url":"https://api.github.com/repos/ceph/ceph/stargazers","contributors_url":"https://api.github.com/repos/ceph/ceph/contributors","subscribers_url":"https://api.github.com/repos/ceph/ceph/subscribers","subscription_url":"https://api.github.com/repos/ceph/ceph/subscription","commits_url":"https://api.github.com/repos/ceph/ceph/commits{/sha}","git_commits_url":"https://api.github.com/repos/ceph/ceph/git/commits{/sha}","comments_url":"https://api.github.com/repos/ceph/ceph/comments{/number}","issue_comment_url":"https://api.github.com/repos/ceph/ceph/issues/comments{/number}","contents_url":"https://api.github.com/repos/ceph/ceph/contents/{+path}","compare_url":"https://api.github.com/repos/ceph/ceph/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ceph/ceph/merges","archive_url":"https://api.github.com/repos/ceph/ceph/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ceph/ceph/downloads","issues_url":"https://api.github.com/repos/ceph/ceph/issues{/number}","pulls_url":"https://api.github.com/repos/ceph/ceph/pulls{/number}","milestones_url":"https://api.github.com/repos/ceph/ceph/milestones{/number}","notifications_url":"https://api.github.com/repos/ceph/ceph/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ceph/ceph/labels{/name}","releases_url":"https://api.github.com/repos/ceph/ceph/releases{/id}","deployments_url":"https://api.github.com/repos/ceph/ceph/deployments","created_at":"2011-09-01T21:41:26Z","updated_at":"2018-04-02T20:10:30Z","pushed_at":"2018-04-02T20:33:56Z","git_url":"git://github.com/ceph/ceph.git","ssh_url":"git@github.com:ceph/ceph.git","clone_url":"https://github.com/ceph/ceph.git","svn_url":"https://github.com/ceph/ceph","homepage":"http://ceph.com","size":276171,"stargazers_count":4287,"watchers_count":4287,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2375,"mirror_url":null,"archived":false,"open_issues_count":705,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":2375,"open_issues":705,"watchers":4287,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ceph/ceph/pulls/19973"},"html":{"href":"https://github.com/ceph/ceph/pull/19973"},"issue":{"href":"https://api.github.com/repos/ceph/ceph/issues/19973"},"comments":{"href":"https://api.github.com/repos/ceph/ceph/issues/19973/comments"},"review_comments":{"href":"https://api.github.com/repos/ceph/ceph/pulls/19973/comments"},"review_comment":{"href":"https://api.github.com/repos/ceph/ceph/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ceph/ceph/pulls/19973/commits"},"statuses":{"href":"https://api.github.com/repos/ceph/ceph/statuses/40b2163ca6ae6ad9e320c023a4d2395d7c87bb24"}},"author_association":"OWNER"}}
{"actor":{"display_login":"gregsfortytwo"}}