{"action":"created","issue":{"url":"https://api.github.com/repos/grpc/grpc/issues/16773","repository_url":"https://api.github.com/repos/grpc/grpc","labels_url":"https://api.github.com/repos/grpc/grpc/issues/16773/labels{/name}","comments_url":"https://api.github.com/repos/grpc/grpc/issues/16773/comments","events_url":"https://api.github.com/repos/grpc/grpc/issues/16773/events","html_url":"https://github.com/grpc/grpc/pull/16773","id":366500295,"node_id":"MDExOlB1bGxSZXF1ZXN0MjIwMTU4Nzc3","number":16773,"title":"Use gRPC thread model (i.e., pollset_set) in ALTS TSI implementation","user":{"login":"yihuazhang","id":26934891,"node_id":"MDQ6VXNlcjI2OTM0ODkx","avatar_url":"https://avatars2.githubusercontent.com/u/26934891?v=4","gravatar_id":"","url":"https://api.github.com/users/yihuazhang","html_url":"https://github.com/yihuazhang","followers_url":"https://api.github.com/users/yihuazhang/followers","following_url":"https://api.github.com/users/yihuazhang/following{/other_user}","gists_url":"https://api.github.com/users/yihuazhang/gists{/gist_id}","starred_url":"https://api.github.com/users/yihuazhang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yihuazhang/subscriptions","organizations_url":"https://api.github.com/users/yihuazhang/orgs","repos_url":"https://api.github.com/users/yihuazhang/repos","events_url":"https://api.github.com/users/yihuazhang/events{/privacy}","received_events_url":"https://api.github.com/users/yihuazhang/received_events","type":"User","site_admin":false},"labels":[{"id":961832073,"node_id":"MDU6TGFiZWw5NjE4MzIwNzM=","url":"https://api.github.com/repos/grpc/grpc/labels/release%20notes:%20no","name":"release notes: no","color":"0f5f75","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-10-03T19:51:22Z","updated_at":"2018-10-05T19:45:21Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/grpc/grpc/pulls/16773","html_url":"https://github.com/grpc/grpc/pull/16773","diff_url":"https://github.com/grpc/grpc/pull/16773.diff","patch_url":"https://github.com/grpc/grpc/pull/16773.patch"},"body":"The PR made the following changes: \r\n\r\n1) refactor `alts_transport_security.{h, cc}` so that it only contains the shared resources that are usable in both gRPC and Envoy (i.e., dedicated CQ) use cases. \r\n2) create `alts_shared_resources.{h, cc}` in `src/core/tsi/alts/handshaker` that contains shared resources and corresponding APIs that are only usable in Envoy use case. \r\n3) use `grpc_channel_create_pollset_set_call` and `grpc_call_start_batch_and_execute` in ALTS handshaker client implementation, when `pollset_set` is passed in from the handshaker. \r\n4) update ALTS handshaker client test to ensure correct inputs (e.g., closure) have been passed when using gRPC thread model.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/grpc/grpc/16773)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/grpc/grpc/issues/comments/427478670","html_url":"https://github.com/grpc/grpc/pull/16773#issuecomment-427478670","issue_url":"https://api.github.com/repos/grpc/grpc/issues/16773","id":427478670,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNzQ3ODY3MA==","user":{"login":"markdroth","id":18664614,"node_id":"MDQ6VXNlcjE4NjY0NjE0","avatar_url":"https://avatars2.githubusercontent.com/u/18664614?v=4","gravatar_id":"","url":"https://api.github.com/users/markdroth","html_url":"https://github.com/markdroth","followers_url":"https://api.github.com/users/markdroth/followers","following_url":"https://api.github.com/users/markdroth/following{/other_user}","gists_url":"https://api.github.com/users/markdroth/gists{/gist_id}","starred_url":"https://api.github.com/users/markdroth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markdroth/subscriptions","organizations_url":"https://api.github.com/users/markdroth/orgs","repos_url":"https://api.github.com/users/markdroth/repos","events_url":"https://api.github.com/users/markdroth/events{/privacy}","received_events_url":"https://api.github.com/users/markdroth/received_events","type":"User","site_admin":false},"created_at":"2018-10-05T19:45:21Z","updated_at":"2018-10-05T19:45:21Z","author_association":"CONTRIBUTOR","body":"> The \"lower-level code\" you mentioned is generic in a sense that it will be invoked by both interested_parties and CQ use case. When it is used for CQ use case, the pollset_set that gets passed in corresponds to the dedicated CQ created in CQ use case. Furthermore, we will only invoke `grpc_call_start_batch_and_execute` within the code by passing in different grpc closures for those two use cases (in the current implementation, we have both `grpc_call_start_batch_and_execute` and `grpc_call_start_batch`).\r\n\r\nYes, that's exactly what I was suggesting.  The lower level will probably wind up looking something like the code we use to make gRPC calls from within the grpclb LB policy:\r\n\r\nhttps://github.com/grpc/grpc/blob/master/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc#L172\r\n\r\nNote that this code very self-contained and does not require a separate event class; it directly owns all of the payload fields (like the send buffer you mentioned) and handles all of the callbacks.\r\n\r\n> 1. `alts_tsi_event` will be different for each batch since its field `send_buffer` will be newly created each time a batch is scheduled. So, after we make `alts_tsi_event` a part of handshaker, we should still  need to initialize the closure each time.\r\n\r\nI agree that there will be a different send buffer for each message sent.  But since you can only have one send_message batch pending on a given call at a given time, this does not require having a different event object for each batch; you can reuse ther same buffer each time.  There should be no need to re-initialize the closures for each batch.\r\n\r\nSee the code I referenced above for an example."}}
{"actor":{"display_login":"markdroth"}}