From: Brian Warner Date: Wed, 22 Oct 2008 05:13:54 +0000 (-0700) Subject: more #514 log-webop status/cancel: add handle-expiration, test coverage X-Git-Url: https://git.rkrishnan.org/COPYING.TGPPL.html?a=commitdiff_plain;h=34ab4e3de37b9f88902af675999f5745e04beb0f;p=tahoe-lafs%2Ftahoe-lafs.git more #514 log-webop status/cancel: add handle-expiration, test coverage --- diff --git a/docs/webapi.txt b/docs/webapi.txt index dfe24995..d72ec2fb 100644 --- a/docs/webapi.txt +++ b/docs/webapi.txt @@ -219,7 +219,9 @@ POST /operations/$HANDLE?t=cancel This terminates the operation, and returns an HTML page explaining what was cancelled. If the operation handle has already expired (see below), this POST will return a 404, which indicates that the operation is no longer - running (either it was completed or terminated). + running (either it was completed or terminated). The response body will be + the same as a t=status on this operation handle, and the handle will be + expired immediately afterwards. The operation handle will eventually expire, to avoid consuming an unbounded amount of memory. The handle's time-to-live can be reset at any time, by diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index 2b0c16fd..251da9cd 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -2150,13 +2150,73 @@ class Web(WebMixin, testutil.StallMixin, unittest.TestCase): client.getPage, url, method="DELETE") return d - def test_bad_ophandle(self): + def test_ophandle_bad(self): url = self.webish_url + "/operations/bogus?t=status" - d = self.shouldHTTPError2("test_bad_ophandle", 400, "400 Bad Request", + d = self.shouldHTTPError2("test_ophandle_bad", 404, "404 Not Found", "unknown/expired handle 'bogus'", client.getPage, url) return d + def test_ophandle_cancel(self): + d = self.POST(self.public_url + "/foo/?t=start-manifest&ophandle=128", + followRedirect=True) + d.addCallback(lambda ignored: + self.GET("/operations/128?t=status&output=JSON")) + def _check1(res): + data = simplejson.loads(res) + self.failUnless("finished" in data, res) + monitor = self.ws.root.child_operations.handles["128"][0] + d = self.POST("/operations/128?t=cancel&output=JSON") + def _check2(res): + data = simplejson.loads(res) + self.failUnless("finished" in data, res) + # t=cancel causes the handle to be forgotten + self.failUnless(monitor.is_cancelled()) + d.addCallback(_check2) + return d + d.addCallback(_check1) + d.addCallback(lambda ignored: + self.shouldHTTPError2("test_ophandle_cancel", + 404, "404 Not Found", + "unknown/expired handle '128'", + self.GET, + "/operations/128?t=status&output=JSON")) + return d + + def test_ophandle_retainfor(self): + d = self.POST(self.public_url + "/foo/?t=start-manifest&ophandle=129&retain-for=60", + followRedirect=True) + d.addCallback(lambda ignored: + self.GET("/operations/129?t=status&output=JSON&retain-for=0")) + def _check1(res): + data = simplejson.loads(res) + self.failUnless("finished" in data, res) + d.addCallback(_check1) + # the retain-for=0 will cause the handle to be expired very soon + d.addCallback(self.stall, 2.0) + d.addCallback(lambda ignored: + self.shouldHTTPError2("test_ophandle_retainfor", + 404, "404 Not Found", + "unknown/expired handle '129'", + self.GET, + "/operations/129?t=status&output=JSON")) + return d + + def test_ophandle_release_after_complete(self): + d = self.POST(self.public_url + "/foo/?t=start-manifest&ophandle=130", + followRedirect=True) + d.addCallback(self.wait_for_operation, "130") + d.addCallback(lambda ignored: + self.GET("/operations/130?t=status&output=JSON&release-after-complete=true")) + # the release-after-complete=true will cause the handle to be expired + d.addCallback(lambda ignored: + self.shouldHTTPError2("test_ophandle_release_after_complete", + 404, "404 Not Found", + "unknown/expired handle '130'", + self.GET, + "/operations/130?t=status&output=JSON")) + return d + def test_incident(self): d = self.POST("/report_incident", details="eek") def _done(res): diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 79dcef39..a7cb43d5 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -353,10 +353,8 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): def _start_operation(self, monitor, renderer, ctx): table = IOpHandleTable(ctx) - ophandle = get_arg(ctx, "ophandle") - assert ophandle - table.add_monitor(ophandle, monitor, renderer) - return table.redirect_to(ophandle, ctx) + table.add_monitor(ctx, monitor, renderer) + return table.redirect_to(ctx) def _POST_start_deep_check(self, ctx): # check this directory and everything reachable from it diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 18765ff3..ea44634d 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -1,24 +1,65 @@ +import time from zope.interface import implements from nevow import rend, url, tags as T from nevow.inevow import IRequest -from twisted.web import html +from twisted.internet import reactor +from twisted.web.http import NOT_FOUND +from twisted.web.html import escape +from twisted.application import service -from allmydata.web.common import IOpHandleTable, get_root, get_arg, WebError +from allmydata.web.common import IOpHandleTable, WebError, \ + get_root, get_arg, boolean_of_arg -class OphandleTable(rend.Page): +MINUTE = 60 +HOUR = 60*MINUTE + +(MONITOR, RENDERER, WHEN_ADDED) = range(3) + +class OphandleTable(rend.Page, service.Service): implements(IOpHandleTable) + UNCOLLECTED_HANDLE_LIFETIME = 1*HOUR + COLLECTED_HANDLE_LIFETIME = 10*MINUTE + def __init__(self): - self.monitors = {} - self.handles = {} + # both of these are indexed by ophandle + self.handles = {} # tuple of (monitor, renderer, when_added) + self.timers = {} + + def stopService(self): + for t in self.timers.values(): + if t.active(): + t.cancel() + del self.handles # this is not restartable + del self.timers + return service.Service.stopService(self) + + def add_monitor(self, ctx, monitor, renderer): + ophandle = get_arg(ctx, "ophandle") + assert ophandle + now = time.time() + self.handles[ophandle] = (monitor, renderer, now) + retain_for = get_arg(ctx, "retain-for", None) + if retain_for is not None: + self._set_timer(ophandle, int(retain_for)) + monitor.when_done().addBoth(self._operation_complete, ophandle) - def add_monitor(self, ophandle, monitor, renderer): - self.monitors[ophandle] = monitor - self.handles[ophandle] = renderer - # TODO: expiration + def _operation_complete(self, res, ophandle): + if ophandle in self.handles: + if ophandle not in self.timers: + # the client has not provided a retain-for= value for this + # handle, so we set our own. + now = time.time() + added = self.handles[ophandle][WHEN_ADDED] + when = max(self.UNCOLLECTED_HANDLE_LIFETIME, now - added) + self._set_timer(ophandle, when) + # if we already have a timer, the client must have provided the + # retain-for= value, so don't touch it. - def redirect_to(self, ophandle, ctx): + def redirect_to(self, ctx): + ophandle = get_arg(ctx, "ophandle") + assert ophandle target = get_root(ctx) + "/operations/" + ophandle + "?t=status" output = get_arg(ctx, "output") if output: @@ -28,14 +69,41 @@ class OphandleTable(rend.Page): def childFactory(self, ctx, name): ophandle = name if ophandle not in self.handles: - raise WebError("unknown/expired handle '%s'" %html.escape(ophandle)) + raise WebError("unknown/expired handle '%s'" % escape(ophandle), + NOT_FOUND) + (monitor, renderer, when_added) = self.handles[ophandle] + t = get_arg(ctx, "t", "status") if t == "cancel": - monitor = self.monitors[ophandle] monitor.cancel() - # return the status anyways + # return the status anyways, but release the handle + self._release_ophandle(ophandle) + + else: + retain_for = get_arg(ctx, "retain-for", None) + if retain_for is not None: + self._set_timer(ophandle, int(retain_for)) + + if monitor.is_finished(): + if boolean_of_arg(get_arg(ctx, "release-after-complete", "false")): + self._release_ophandle(ophandle) + if retain_for is None: + # this GET is collecting the ophandle, so change its timer + self._set_timer(ophandle, self.COLLECTED_HANDLE_LIFETIME) + + return renderer + + def _set_timer(self, ophandle, when): + if ophandle in self.timers and self.timers[ophandle].active(): + self.timers[ophandle].cancel() + t = reactor.callLater(when, self._release_ophandle, ophandle) + self.timers[ophandle] = t - return self.handles[ophandle] + def _release_ophandle(self, ophandle): + if ophandle in self.timers and self.timers[ophandle].active(): + self.timers[ophandle].cancel() + self.timers.pop(ophandle, None) + self.handles.pop(ophandle, None) class ReloadMixin: diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py index 26dfe1a0..115feb78 100644 --- a/src/allmydata/web/root.py +++ b/src/allmydata/web/root.py @@ -118,11 +118,14 @@ class Root(rend.Page): addSlash = True docFactory = getxmlfile("welcome.xhtml") + def __init__(self, original=None): + rend.Page.__init__(self, original) + self.child_operations = operations.OphandleTable() + child_uri = URIHandler() child_cap = URIHandler() child_file = FileHandler() child_named = FileHandler() - child_operations = operations.OphandleTable() child_webform_css = webform.defaultCSS child_tahoe_css = nevow_File(resource_filename('allmydata.web', 'tahoe.css')) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 2329515d..0b3add8c 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -131,6 +131,7 @@ class WebishServer(service.MultiService): self.site.requestFactory = MyRequest if self.root.child_operations: self.site.remember(self.root.child_operations, IOpHandleTable) + self.root.child_operations.setServiceParent(self) s = strports.service(webport, site) s.setServiceParent(self) self.listener = s # stash it so the tests can query for the portnum