( )⚙ D2860 wireproto: buffer output frames when in half duplex mode

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2860

wireproto: buffer output frames when in half duplex mode
ClosedPublic

Authored by indygreg on Mar 14 2018, 12:43 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG861e9d37e56e: wireproto: buffer output frames when in half duplex mode
Summary

Previously, when told that a response was ready, the server reactor
would instruct the caller to send frames immediately. This was OK
as an initial implementation. But it would not work for half-duplex
connections where the sender can't receive until all data has been
transmitted - such as httplib based clients.

In this commit, we teach the reactor that output frames should
be buffered until end of input is seen. This required a new
event to inform the reactor of end of input. The result from that
event will instruct the consumer to send all buffered frames.

The HTTP server is buffered by default.

This change effectively hides the complexity of buffering within
the reactor so that transports need not be concerned about it.
This helps keep the transports "dumb" and will make implementing
multiple requests-responses per atomic exchange (like an HTTP
request) much simpler.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Mar 14 2018, 12:43 PM
indygreg updated this revision to Diff 7144.Mar 19 2018, 7:59 PM
durin42 added a subscriber: durin42.Mar 21 2018, 9:18 PM
durin42 added inline comments.
mercurial/wireprotoserver.py
418–422

Recent RFCs explicitly disallow early responses with a 2xx series status code. Probably not worth pursuing.

durin42 accepted this revision.Mar 21 2018, 9:32 PM
This revision is now accepted and ready to land.Mar 21 2018, 9:32 PM
Closed by commit rHG861e9d37e56e: wireproto: buffer output frames when in half duplex mode (authored by indygreg). · Explain WhyMar 21 2018, 9:40 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 7232

mercurial/wireprotoframing.py

runcommand runcommand
Indicates that the consumer should run a wire protocol command. Details Indicates that the consumer should run a wire protocol command. Details
of the command to run are given in the data structure. of the command to run are given in the data structure.
wantframe wantframe
Indicates that nothing of interest happened and the server is waiting on Indicates that nothing of interest happened and the server is waiting on
more frames from the client before anything interesting can be done. more frames from the client before anything interesting can be done.
noop
Indicates no additional action is required.
""" """
def __init__(self): def __init__(self, deferoutput=False):
"""Construct a new server reactor.
``deferoutput`` can be used to indicate that no output frames should be
instructed to be sent until input has been exhausted. In this mode,
events that would normally generate output frames (such as a command
response being ready) will instead defer instructing the consumer to
send those frames. This is useful for half-duplex transports where the
sender cannot receive until all data has been transmitted.
"""
self._deferoutput = deferoutput
self._state = 'idle' self._state = 'idle'
self._bufferedframegens = []
self._activecommand = None self._activecommand = None
self._activeargs = None self._activeargs = None
self._activedata = None self._activedata = None
self._expectingargs = None self._expectingargs = None
self._expectingdata = None self._expectingdata = None
self._activeargname = None self._activeargname = None
self._activeargchunks = None self._activeargchunks = None
return meth(frametype, frameflags, payload) return meth(frametype, frameflags, payload)
def onbytesresponseready(self, data): def onbytesresponseready(self, data):
"""Signal that a bytes response is ready to be sent to the client. """Signal that a bytes response is ready to be sent to the client.
The raw bytes response is passed as an argument. The raw bytes response is passed as an argument.
""" """
framegen = createbytesresponseframesfrombytes(data)
if self._deferoutput:
self._bufferedframegens.append(framegen)
return 'noop', {}
else:
return 'sendframes', {
'framegen': framegen,
}
def oninputeof(self):
"""Signals that end of input has been received.
No more frames will be received. All pending activity should be
completed.
"""
if not self._deferoutput or not self._bufferedframegens:
return 'noop', {}
# If we buffered all our responses, emit those.
def makegen():
for gen in self._bufferedframegens:
for frame in gen:
yield frame
return 'sendframes', { return 'sendframes', {
'framegen': createbytesresponseframesfrombytes(data), 'framegen': makegen(),
} }
def onapplicationerror(self, msg): def onapplicationerror(self, msg):
return 'sendframes', { return 'sendframes', {
'framegen': createerrorframe(msg, application=True), 'framegen': createerrorframe(msg, application=True),
} }
def _makeerrorresult(self, msg): def _makeerrorresult(self, msg):

mercurial/wireprotoserver.py

frametype, frameflags, payload = frame frametype, frameflags, payload = frame
states.append(b'received: %d %d %s' % (frametype, frameflags, payload)) states.append(b'received: %d %d %s' % (frametype, frameflags, payload))
action, meta = reactor.onframerecv(frametype, frameflags, payload) action, meta = reactor.onframerecv(frametype, frameflags, payload)
states.append(json.dumps((action, meta), sort_keys=True, states.append(json.dumps((action, meta), sort_keys=True,
separators=(', ', ': '))) separators=(', ', ': ')))
action, meta = reactor.oninputeof()
meta['action'] = action
states.append(json.dumps(meta, sort_keys=True, separators=(', ',': ')))
res.status = b'200 OK' res.status = b'200 OK'
res.headers[b'Content-Type'] = b'text/plain' res.headers[b'Content-Type'] = b'text/plain'
res.setbodybytes(b'\n'.join(states)) res.setbodybytes(b'\n'.join(states))
def _processhttpv2request(ui, repo, req, res, authedperm, reqcommand, proto): def _processhttpv2request(ui, repo, req, res, authedperm, reqcommand, proto):
"""Post-validation handler for HTTPv2 requests. """Post-validation handler for HTTPv2 requests.
Called when the HTTP request contains unified frame-based protocol Called when the HTTP request contains unified frame-based protocol
frames for evaluation. frames for evaluation.
""" """
reactor = wireprotoframing.serverreactor() # TODO Some HTTP clients are full duplex and can receive data before
# the entire request is transmitted. Figure out a way to indicate support
# for that so we can opt into full duplex mode.
reactor = wireprotoframing.serverreactor(deferoutput=True)
seencommand = False seencommand = False
durin42Unsubmitted
Not Done

Recent RFCs explicitly disallow early responses with a 2xx series status code. Probably not worth pursuing.

durin42: Recent RFCs explicitly disallow early responses with a 2xx series status code. Probably not…
while True: while True:
frame = wireprotoframing.readframe(req.bodyfh) frame = wireprotoframing.readframe(req.bodyfh)
if not frame: if not frame:
break break
action, meta = reactor.onframerecv(*frame) action, meta = reactor.onframerecv(*frame)
res.status = b'200 OK' res.status = b'200 OK'
res.headers[b'Content-Type'] = b'text/plain' res.headers[b'Content-Type'] = b'text/plain'
res.setbodybytes(meta['message'] + b'\n') res.setbodybytes(meta['message'] + b'\n')
return return
else: else:
raise error.ProgrammingError( raise error.ProgrammingError(
'unhandled action from frame processor: %s' % action) 'unhandled action from frame processor: %s' % action)
action, meta = reactor.oninputeof()
if action == 'sendframes':
# We assume we haven't started sending the response yet. If we're
# wrong, the response type will raise an exception.
res.status = b'200 OK'
res.headers[b'Content-Type'] = FRAMINGTYPE
res.setbodygen(meta['framegen'])
elif action == 'noop':
pass
else:
raise error.ProgrammingError('unhandled action from frame processor: %s'
% action)
def _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand, reactor, def _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand, reactor,
command): command):
"""Dispatch a wire protocol command made from HTTPv2 requests. """Dispatch a wire protocol command made from HTTPv2 requests.
The authenticated permission (``authedperm``) along with the original The authenticated permission (``authedperm``) along with the original
command from the URL (``reqcommand``) are passed in. command from the URL (``reqcommand``) are passed in.
""" """
# We already validated that the session has permissions to perform the # We already validated that the session has permissions to perform the
if isinstance(rsp, wireprototypes.bytesresponse): if isinstance(rsp, wireprototypes.bytesresponse):
action, meta = reactor.onbytesresponseready(rsp.data) action, meta = reactor.onbytesresponseready(rsp.data)
else: else:
action, meta = reactor.onapplicationerror( action, meta = reactor.onapplicationerror(
_('unhandled response type from wire proto command')) _('unhandled response type from wire proto command'))
if action == 'sendframes': if action == 'sendframes':
res.setbodygen(meta['framegen']) res.setbodygen(meta['framegen'])
elif action == 'noop':
pass
else: else:
raise error.ProgrammingError('unhandled event from reactor: %s' % raise error.ProgrammingError('unhandled event from reactor: %s' %
action) action)
# Maps API name to metadata so custom API can be registered. # Maps API name to metadata so custom API can be registered.
API_HANDLERS = { API_HANDLERS = {
HTTPV2: { HTTPV2: {
'config': ('experimental', 'web.api.http-v2'), 'config': ('experimental', 'web.api.http-v2'),

tests/test-http-api-httpv2.t

s> host: $LOCALIP:$HGPORT\r\n (glob) s> host: $LOCALIP:$HGPORT\r\n (glob)
s> \r\n s> \r\n
s> \x08\x00\x00\x12command1\x0b\x00\x00 \x03\x00\x04\x00fooval1\x0b\x00\x00"\x04\x00\x03\x00bar1val s> \x08\x00\x00\x12command1\x0b\x00\x00 \x03\x00\x04\x00fooval1\x0b\x00\x00"\x04\x00\x03\x00bar1val
s> makefile('rb', None) s> makefile('rb', None)
s> HTTP/1.1 200 OK\r\n s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n s> Date: $HTTP_DATE$\r\n
s> Content-Type: text/plain\r\n s> Content-Type: text/plain\r\n
s> Content-Length: 291\r\n s> Content-Length: 310\r\n
s> \r\n s> \r\n
s> received: 1 2 command1\n s> received: 1 2 command1\n
s> ["wantframe", {"state": "command-receiving-args"}]\n s> ["wantframe", {"state": "command-receiving-args"}]\n
s> received: 2 0 \x03\x00\x04\x00fooval1\n s> received: 2 0 \x03\x00\x04\x00fooval1\n
s> ["wantframe", {"state": "command-receiving-args"}]\n s> ["wantframe", {"state": "command-receiving-args"}]\n
s> received: 2 2 \x04\x00\x03\x00bar1val\n s> received: 2 2 \x04\x00\x03\x00bar1val\n
s> ["runcommand", {"args": {"bar1": "val", "foo": "val1"}, "command": "command1", "data": null}]\n s> ["runcommand", {"args": {"bar1": "val", "foo": "val1"}, "command": "command1", "data": null}]\n
s> received: <no frame> s> received: <no frame>\n
s> {"action": "noop"}
$ cat error.log $ cat error.log

tests/test-wireproto-serverreactor.py

from __future__ import absolute_import, print_function from __future__ import absolute_import, print_function
import unittest import unittest
from mercurial import ( from mercurial import (
util, util,
wireprotoframing as framing, wireprotoframing as framing,
) )
ffs = framing.makeframefromhumanstring ffs = framing.makeframefromhumanstring
def makereactor(): def makereactor(deferoutput=False):
return framing.serverreactor() return framing.serverreactor(deferoutput=deferoutput)
def sendframes(reactor, gen): def sendframes(reactor, gen):
"""Send a generator of frame bytearray to a reactor. """Send a generator of frame bytearray to a reactor.
Emits a generator of results from ``onframerecv()`` calls. Emits a generator of results from ``onframerecv()`` calls.
""" """
for frame in gen: for frame in gen:
frametype, frameflags, framelength = framing.parseheader(frame) frametype, frameflags, framelength = framing.parseheader(frame)
self.assertEqual(len(results), 1) self.assertEqual(len(results), 1)
self.assertaction(results[0], 'runcommand') self.assertaction(results[0], 'runcommand')
self.assertEqual(results[0][1], { self.assertEqual(results[0][1], {
'command': b'mycommand', 'command': b'mycommand',
'args': {}, 'args': {},
'data': None, 'data': None,
}) })
result = reactor.oninputeof()
self.assertaction(result, 'noop')
def test1argument(self): def test1argument(self):
reactor = makereactor() reactor = makereactor()
results = list(sendcommandframes(reactor, b'mycommand', results = list(sendcommandframes(reactor, b'mycommand',
{b'foo': b'bar'})) {b'foo': b'bar'}))
self.assertEqual(len(results), 2) self.assertEqual(len(results), 2)
self.assertaction(results[0], 'wantframe') self.assertaction(results[0], 'wantframe')
self.assertaction(results[1], 'runcommand') self.assertaction(results[1], 'runcommand')
self.assertEqual(results[1][1], { self.assertEqual(results[1][1], {
list(sendcommandframes(reactor, b'mycommand', {})) list(sendcommandframes(reactor, b'mycommand', {}))
result = reactor.onapplicationerror(b'some message') result = reactor.onapplicationerror(b'some message')
self.assertaction(result, 'sendframes') self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [ self.assertframesequal(result[1]['framegen'], [
b'error-response application some message', b'error-response application some message',
]) ])
def test1commanddeferresponse(self):
"""Responses when in deferred output mode are delayed until EOF."""
reactor = makereactor(deferoutput=True)
results = list(sendcommandframes(reactor, b'mycommand', {}))
self.assertEqual(len(results), 1)
self.assertaction(results[0], 'runcommand')
result = reactor.onbytesresponseready(b'response')
self.assertaction(result, 'noop')
result = reactor.oninputeof()
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
b'bytes-response eos response',
])
def testmultiplecommanddeferresponse(self):
reactor = makereactor(deferoutput=True)
list(sendcommandframes(reactor, b'command1', {}))
list(sendcommandframes(reactor, b'command2', {}))
result = reactor.onbytesresponseready(b'response1')
self.assertaction(result, 'noop')
result = reactor.onbytesresponseready(b'response2')
self.assertaction(result, 'noop')
result = reactor.oninputeof()
self.assertaction(result, 'sendframes')
self.assertframesequal(result[1]['framegen'], [
b'bytes-response eos response1',
b'bytes-response eos response2'
])
if __name__ == '__main__': if __name__ == '__main__':
import silenttestrunner import silenttestrunner
silenttestrunner.main(__name__) silenttestrunner.main(__name__)