[SERVER-27359] Create a Timeout class for use in the server Created: 09/Dec/16 Updated: 09/Oct/19 Resolved: 09/Oct/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Samantha Ritter (Inactive) | Assignee: | Mira Carey |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | platforms-re-triaged | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Participants: |
| Description |
|
We currently have no consistent way of passing timeouts around in the server. Some places pass timeouts as Seconds or Milliseconds, some pass timeouts as bare doubles and rely on code comments to state what granularity they express. Some methods use boost::optional to express whether a timeout has been set, and some provide a default value instead, but do this inconsistently (we alternate between using "0" and "-1" to express "no timeout specified"). All these inconsistencies make it very easy to create bugs like We should implement a Timeout class to uniformly express timeouts in the server. This class should be flexible enough to accept Seconds or Milliseconds. Because we rely on "explicit passed-in value or a default" logic, this class should also express optionality in a sane way. |
| Comments |
| Comment by Mira Carey [ 09/Oct/19 ] |
|
Agree with schwerin here, it's not obvious to me why we'd want a timeout type distinct from our existing durations. To the extent to which we used to mix up variables with different units, durations have fixed this problem. To the extent to which it's hard to have a uniform global timeout, durations are the wrong unit type (we'd rather set a deadline and synthesize timeouts if/when we really need them). |
| Comment by Andy Schwerin [ 09/Dec/16 ] |
|
First, I'm very excited to do a better job dealing with timeouts in our code. I'm curious why we need more than strongly typed durations for this purpose. Are there cases where a timeout is expressed as a Seconds type and we use its count as though it were Milliseconds? The idiom for getting counts out of durations in our codebase is to use durationCount<DesiredDurationType>(durationValue). Calls to count() on a duration are almost always errors, but easy to see in code, and not likely to be fixed by having a type called Timeout. So could we be using SomeDuration or optional<SomeDuration> for timeouts everywhere, and just fix up our parsing and consuming points, which we'd have to do in any event to use a Timeout class? |