{"action":"created","issue":{"url":"https://api.github.com/repos/ceph/ceph/issues/24693","repository_url":"https://api.github.com/repos/ceph/ceph","labels_url":"https://api.github.com/repos/ceph/ceph/issues/24693/labels{/name}","comments_url":"https://api.github.com/repos/ceph/ceph/issues/24693/comments","events_url":"https://api.github.com/repos/ceph/ceph/issues/24693/events","html_url":"https://github.com/ceph/ceph/pull/24693","id":372444763,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI0NjI0MDA5","number":24693,"title":"msg/async: fix is_queued() semantics","user":{"login":"idryomov","id":567341,"node_id":"MDQ6VXNlcjU2NzM0MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/567341?v=4","gravatar_id":"","url":"https://api.github.com/users/idryomov","html_url":"https://github.com/idryomov","followers_url":"https://api.github.com/users/idryomov/followers","following_url":"https://api.github.com/users/idryomov/following{/other_user}","gists_url":"https://api.github.com/users/idryomov/gists{/gist_id}","starred_url":"https://api.github.com/users/idryomov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/idryomov/subscriptions","organizations_url":"https://api.github.com/users/idryomov/orgs","repos_url":"https://api.github.com/users/idryomov/repos","events_url":"https://api.github.com/users/idryomov/events{/privacy}","received_events_url":"https://api.github.com/users/idryomov/received_events","type":"User","site_admin":false},"labels":[{"id":134774049,"node_id":"MDU6TGFiZWwxMzQ3NzQwNDk=","url":"https://api.github.com/repos/ceph/ceph/labels/bug%20fix","name":"bug fix","color":"ac0000","default":false},{"id":144329561,"node_id":"MDU6TGFiZWwxNDQzMjk1NjE=","url":"https://api.github.com/repos/ceph/ceph/labels/core","name":"core","color":"0041a2","default":false},{"id":440630784,"node_id":"MDU6TGFiZWw0NDA2MzA3ODQ=","url":"https://api.github.com/repos/ceph/ceph/labels/wip-rdias-testing","name":"wip-rdias-testing","color":"fef2c0","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-10-22T09:06:55Z","updated_at":"2018-10-23T10:32:15Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/ceph/ceph/pulls/24693","html_url":"https://github.com/ceph/ceph/pull/24693","diff_url":"https://github.com/ceph/ceph/pull/24693.diff","patch_url":"https://github.com/ceph/ceph/pull/24693.patch"},"body":"Before AsyncConnection was split into two classes as part of the\r\nmulti-protocol refactor, we only had AsyncConnection::is_queued().\r\nIt checked both out_q and outcoming_bl because out_q was part of\r\nAsyncConnection.\r\n\r\nout_q is now part of ProtocolV1.  AsyncConnection should no longer be\r\nconcerned with out_q, only with outcoming_bl.  Checking whether out_q\r\nis empty in _try_send() is particuarly wrong because if the write is\r\nfininished (i.e. outcoming_bl is empty) but out_q has something in it,\r\nthe write callback isn't invoked.\r\n\r\nAlthough probably not strictly necessary, this commit preserves the\r\nsemantics of connection->is_queued() in Protocol.cc.\r\n\r\nSigned-off-by: Ilya Dryomov <idryomov@gmail.com>\r\n\r\n\r\n<!--\r\nThank you for opening a pull request!  Here are some tips on creating\r\na well formatted contribution.\r\n\r\nPlease give your pull request a title like \"[component]: [short description]\"\r\n\r\nThis is the format for commit messages:\r\n\r\n\"\"\"\r\n[component]: [short description]\r\n\r\n[A longer multiline description]\r\n\r\nFixes: [ticket URL on tracker.ceph.com, create one if necessary]\r\nSigned-off-by: [Your Name] <[your email]>\r\n\"\"\"\r\n\r\nThe Signed-off-by line is important, and it is your certification that\r\nyour contributions satisfy the Developers Certificate or Origin.  For\r\nmore detail, see SubmittingPatches.rst.\r\n\r\nThe component is the short name of a major daemon or subsystem,\r\nsomething like \"mon\", \"osd\", \"mds\", \"rbd, \"rgw\", etc. For ceph-mgr modules,\r\ngive the component as \"mgr/<module name>\" rather than a path into pybind.\r\n\r\nFor more examples, simply use \"git log\" and look at some historical commits.\r\n\r\nThis was just a quick overview.  More information for contributors is available here:\r\nhttps://raw.githubusercontent.com/ceph/ceph/master/SubmittingPatches.rst\r\n\r\n-->\r\n\r\n- [ ] References tracker ticket\r\n- [ ] Updates documentation if necessary\r\n- [ ] Includes tests for new functionality or reproducer for bug\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/ceph/ceph/issues/comments/432192460","html_url":"https://github.com/ceph/ceph/pull/24693#issuecomment-432192460","issue_url":"https://api.github.com/repos/ceph/ceph/issues/24693","id":432192460,"node_id":"MDEyOklzc3VlQ29tbWVudDQzMjE5MjQ2MA==","user":{"login":"rjfd","id":17119885,"node_id":"MDQ6VXNlcjE3MTE5ODg1","avatar_url":"https://avatars0.githubusercontent.com/u/17119885?v=4","gravatar_id":"","url":"https://api.github.com/users/rjfd","html_url":"https://github.com/rjfd","followers_url":"https://api.github.com/users/rjfd/followers","following_url":"https://api.github.com/users/rjfd/following{/other_user}","gists_url":"https://api.github.com/users/rjfd/gists{/gist_id}","starred_url":"https://api.github.com/users/rjfd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rjfd/subscriptions","organizations_url":"https://api.github.com/users/rjfd/orgs","repos_url":"https://api.github.com/users/rjfd/repos","events_url":"https://api.github.com/users/rjfd/events{/privacy}","received_events_url":"https://api.github.com/users/rjfd/received_events","type":"User","site_admin":false},"created_at":"2018-10-23T10:32:15Z","updated_at":"2018-10-23T10:32:15Z","author_association":"CONTRIBUTOR","body":"> I hit it several times after the refactor (all it takes is a partial send when the write callback is set) and not once before the refactor. I think it's just that the previous code didn't condition the write callback on that. It didn't have such write_callback_handler at all...\r\n\r\nThe previous code didn't have a write_callback_handler but it used the `state_after_send` variable to specify the next state to transition after all bytes were written to the socket.\r\nAnd the condition used to schedule the function that uses the `state_after_send` is exactly the same as in the refactored code.\r\n\r\nWhat I think it happened was that in the old code the function that handled the partial write state `process_connection` was also called for every read event (besides the complete write events), and that would unblock the state. Opposed to what happens now that there is a specific function to handle the complete write events."}}
{"actor":{"display_login":"rjfd"}}