[SERVER-53361] improve invariant(): split up compound invariant calls, show comparison args Created: 15/Dec/20 Updated: 06/Dec/22 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Billy Donahue | Assignee: | Backlog - Service Architecture |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | sa-remove-fv-backlog-22 | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Assigned Teams: |
Service Arch
|
||||
| Participants: | |||||
| Description |
|
Invariant is extremely primitive, and yields very litte information. It's not good to have an invariant checking on more than one thing like this. "On one of the loop runthroughs, we receive an invariant beginning here at the top of the stack trace implying at the client.cpp level that the just-created opCtx already exists on the thread's client, or that the just-created opCtx has been created as nullptr."
We should not have to waste time deducing which condition fired. I recommend we make an effort to find all invariants that are compound conditionals and consider separating them. A follow-on idea: make "invariant" aware that it is testing a composition of conditions. There are 93 matches to `git grep invariant(.*\&\&`. It would also be extremely useful to see the left and right arguments to relational operators in failed invariants. I think the C++ argument passed to invariant can be interrogated for not only its bool value, but also for a description, including its inputs. Instead of: invariant(a > lower && a < upper); You might soon write something like the following:
I think this, or something like it, is technically feasible. Boost.Phoenix does this sort of evaluation with the option to do subexpression decomposition. It looks like it could be leveraged to get access to the intermediate subexpression results. That would be an advanced case for converting our existing invariants. It doesn't have to go that far. It would be enough of a leap forward to just SEE the inputs, which could be done with a fairly standard macro implementation. INVARIANT_BIND_EXPR is just one idea. The important thing is to give invariant a way to statically determine if if condition argument can provide a description. This would enable users to extend the kinds of invariants we can make, without modifying invariant.h and without requiring new macros. |