I want results to stick around until I explicitly tell them to go away (not: "unknown/expired handle")

[zooko]

I ran a deep-verify operation, and when I hit "reload" to find out how it was going, I got:

unknown/expired handle 'xejkkdqay2ygimef2huncyxyci'

Fortunately in this case the operation I was performing was side-effect free, so this only costs my time. But what if it had been a side-effecting operation such as a deep repair? Then it would be impossible for me to find out what happened.

I know that I originally agreed to this time-out mechanism, but now I'm dissatisfied with it. What about adding a button to the form which says "Free up these results" and then storing the results until that button is pressed or the node is restarted?

[zooko]

Making the Subject of this ticket a bit more specific -- I'm about to open another ticket with a different aspect of this problem.

[zooko]

As an interim measure -- even if we don't go to a "keep this until I tell you to let go of it" design -- could we please set the timeouts to approximately one hundred times as long as they currently are? Maybe 100 hours for uncollected results and 1000 minutes for collected results?

[davidsarah]

#949 was a duplicate:

I just ran an important deep-check-and-repair, and when I came back to see what had happened I got "unknown/expired handle". This information is gone for good -- since the repair part repaired the files now there is now way for me to tell what their state was before the repair, which was information that I very much wanted to know.

I propose that operation handles be infinite duration by default, and people who want to save a few KB of virtual memory at the cost of risking losing information can configure their node to expire operation handles.

[davidsarah]

Increasing the timeout would be very easy.

[kevan]

I can take care of this.

[kevan]

I'm attaching four patches that fix this.

• 577behavior.darcspatch.txt is the easy fix to the problem identified in this ticket.
• 577docs.darcspatch.txt updates webapi.txt to use the new expiration values.
• 577testmodifications.darcspatch.txt modifies the OphandleTable​ class to use a provided twisted.internet.task.Clock for callLater calls instead of the reactor, if such a clock is provided. It also changes some code in other classes to relay the provided clock to the OphandleTable?. To effectively test this fix, I needed a deterministic way to simulate the passage of large periods of time (since it is very probably unacceptable to have the test suite take 100 hours to run), and I gather that using twisted.internet.task.Clock is the way to do that. That said, I'm by no means a twisted expert, so if there's an easier way of doing that that I missed, please let me know.
• 577tests.darcspatch.txt adds tests for my changes.

I'm happy with where these are, so I'll add the review-needed keyword.

[davidsarah]

100 hours and 1000 minutes require some mental arithmetic to grasp how long they are. How about 4 days and 1 day respectively (96 hours and 1440 minutes)?

In webapi.txt: "operations which have finished" -> "operations that have finished".

Nitpick: single blank line between test methods.

[kevan]

Replying to davidsarah:

100 hours and 1000 minutes require some mental arithmetic to grasp how long they are. How about 4 days and 1 day respectively (96 hours and 1440 minutes)?

That's much nicer to think about (and read). I updated my patches to use those.

In webapi.txt: "operations which have finished" -> "operations that have finished".

Fixed.

Nitpick: single blank line between test methods.

Fixed.

Thanks for the feedback.

[zooko]

What's the status of this ticket? I think it is review-needed and that David-Sarah or someone should look at the latest versions of Kevan's patches.

[davidsarah]

Reviewed and fixed in e9b59a4949d3237a, 09177a6e19af6e8f, bcdc78bd446f28e2, 81ad52d6ebf05f4f.