I'd like to get clearance on the changes before going to the trouble of testing ...
13 years, 7 months ago
(2010-10-24 03:24:06 UTC)
#3
I'd like to get clearance on the changes before going to the trouble
of testing them.
The existing tests test this indirectly by testing the DefaultServer,
so it'll just be adding a level of indirection to them.
On 24 October 2010 14:15, Rob 'Commander' Pike <r@google.com> wrote:
> surely there are tests too?
>
> -rob
>
>
In general, I feel this is on the right path. I wouldn't be surprised if ...
13 years, 7 months ago
(2010-10-25 20:36:41 UTC)
#6
In general, I feel this is on the right path.
I wouldn't be surprised if something had to be added to interact externally with
the serviceMap at some point.
Removing the path from the CONNECT error is probably fine, though again, I
wouldn't be surprised if one needed to end up customizing/accessing the
connected var.
On 2010/10/25 04:34:13, adg wrote:
> Hello r, rsc, msolo (cc:
http://mail.google.com/mail/?view=cm&fs=1&tf=1&to=golang-dev@googlegroups.com...),
>
> I'd like you to review this change.
I had sent this email to rsc earlier. Do you think you'll be able to ...
13 years, 7 months ago
(2010-10-25 21:13:17 UTC)
#7
I had sent this email to rsc earlier. Do you think you'll be able to break up
the input() function such that it can synchronously serve a single request?
I'm currently working with msolo on a server project. I've been trying to write
a HTTP POST based handler for jsonrpc, and possibly other protocols too.
Basically, we want to send each RPC request as an HTTP POST where the contents
of the request are in the Body.
The way I tried to do this is by creating a proxy object that masquerades as a
connection that the jsonrpc codec expects:
// JsonHttpBroker implements the ReadWriteCloser interface for the Json HTTP
server.
type JsonHttpBroker struct {
io.Writer // Initialize with http.ResponseWriter
io.ReadCloser // Initialize with http.Request's Body member
}
You can find the code in /home/sougou/jsonhttprpc.go
But this code doesn't work. This is what we think the reason is:
1. ServeJsonHttpRpc (HTTP handler) eventually ends up calling input() in
rpc/server.go
2. The input function receives the request correctly from the http.Request's
Body member.
3. It launches the go routine to execute the rpc call.
4. It loops back to read the next request, and receives an EOF from the Body.
5. It exits the loop, closes the connection, and returns to ServeJsonHttpRpc.
6. ServeJsonHttpRpc returns back to the http library, which ends the request.
However, all this happens before the go routine (step 3) finishes writing the
data on the ResponseWriter. This results in the client receiving an empty
response.
In other words, we need some way/mechanism to wait for step 3 to finish before
ServeJsonHttpRpc can return back to the caller.
Some of the options we've brainstormed, in order of decreasing excitement:
1. Pull out parts of the input() function to make them individually reusable.
We'll basically need something we can call that will process one request without
launching a go routine. So, the function will return only after the rpc call has
completed.
2. Add some synchronization mechanism that can be used to wait for the go
routine to complete, maybe a channel that ServeJsonHttpRpc can wait on.
3. Make the function map public, so ServeJsonHttpRpc can do its own version of
input.
On 2010/10/25 20:36:41, msolo wrote:
> In general, I feel this is on the right path.
>
> I wouldn't be surprised if something had to be added to interact externally
with
> the serviceMap at some point.
>
> Removing the path from the CONNECT error is probably fine, though again, I
> wouldn't be surprised if one needed to end up customizing/accessing the
> connected var.
>
> On 2010/10/25 04:34:13, adg wrote:
> > Hello r, rsc, msolo (cc:
>
http://mail.google.com/mail/?view=cm&fs=1&tf=1&to=golang-dev%40googlegroups.c...),
> >
> > I'd like you to review this change.
time for tests http://codereview.appspot.com/2696041/diff/14001/src/pkg/rpc/server.go File src/pkg/rpc/server.go (right): http://codereview.appspot.com/2696041/diff/14001/src/pkg/rpc/server.go#newcode175 src/pkg/rpc/server.go:175: type Server struct { doc comment ...
13 years, 7 months ago
(2010-10-25 23:43:25 UTC)
#8
On 26 October 2010 08:13, <sougou@google.com> wrote: > I had sent this email to rsc ...
13 years, 7 months ago
(2010-10-25 23:54:58 UTC)
#9
On 26 October 2010 08:13, <sougou@google.com> wrote:
> I had sent this email to rsc earlier. Do you think you'll be able to
> break up the input() function such that it can synchronously serve a
> single request?
>
> I'm currently working with msolo on a server project. I've been trying
> to write a HTTP POST based handler for jsonrpc, and possibly other
> protocols too.
> Basically, we want to send each RPC request as an HTTP POST where the
> contents of the request are in the Body.
> The way I tried to do this is by creating a proxy object that
> masquerades as a connection that the jsonrpc codec expects:
>
> // JsonHttpBroker implements the ReadWriteCloser interface for the Json
> HTTP server.
> type JsonHttpBroker struct {
> io.Writer // Initialize with http.ResponseWriter
> io.ReadCloser // Initialize with http.Request's Body member
> }
> You can find the code in /home/sougou/jsonhttprpc.go
>
> But this code doesn't work. This is what we think the reason is:
> 1. ServeJsonHttpRpc (HTTP handler) eventually ends up calling input() in
> rpc/server.go
> 2. The input function receives the request correctly from the
> http.Request's Body member.
> 3. It launches the go routine to execute the rpc call.
> 4. It loops back to read the next request, and receives an EOF from the
> Body.
> 5. It exits the loop, closes the connection, and returns to
> ServeJsonHttpRpc.
> 6. ServeJsonHttpRpc returns back to the http library, which ends the
> request.
>
> However, all this happens before the go routine (step 3) finishes
> writing the data on the ResponseWriter. This results in the client
> receiving an empty response.
> In other words, we need some way/mechanism to wait for step 3 to finish
> before ServeJsonHttpRpc can return back to the caller.
I just took a crack at separating (what is now called) ServeCodec into
two functions. ServeCodec calls a new method ReadRequest to read in
the request, and then makes the service.call() in a new goroutine.
ReadRequest returns a *service, argv, replyv, req, and error state. It
gives you all you need to make the service.call(), and that would suit
your "one-shot" approach.
This approach requires exporting the service and methodType types, and
possibly more. Things then start to get more complicated than I'm
comfortable with (I'm already complicating rpc more with this CL). I
think with some more thought and some judicious refactoring, the
problem could be split a little neater.
> Some of the options we've brainstormed, in order of decreasing
> excitement:
> 1. Pull out parts of the input() function to make them individually
> reusable. We'll basically need something we can call that will process
> one request without launching a go routine. So, the function will return
> only after the rpc call has completed.
This is what I tried, and I think this would be the way to go if we
were to make further changes to rpc.
> 2. Add some synchronization mechanism that can be used to wait for the
> go routine to complete, maybe a channel that ServeJsonHttpRpc can wait
> on.
> 3. Make the function map public, so ServeJsonHttpRpc can do its own
> version of input.
I haven't thought much about the implications of these, but they're
not very appealing from the outset.
Andrew
Issue 2696041: code review 2696041: rpc: expose Server type to allow multiple RPC Server in...
(Closed)
Created 13 years, 7 months ago by adg
Modified 13 years, 6 months ago
Reviewers:
Base URL:
Comments: 8
