[SERVER-40540] Validate the parameters to configureFailPoint given the failPoint being configured. Created: 08/Apr/19 Updated: 28/Mar/23 Resolved: 28/Mar/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | 4.1.9 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Oleg Pudeyev (Inactive) | Assignee: | Backlog - Service Architecture |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | devtools-to-servicearch, re-triaged-ticket | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Service Arch
|
| Participants: |
| Description |
|
For example, if a "failCommand" falpoint is configured with configureFailPoint but without a command to fail, the operation succeeds:
However, since no fail point was set as a result, this command should have failed with an appropriate error message indicating the requirement to supply a command to fail. |
| Comments |
| Comment by Lauren Lewis (Inactive) [ 24/Feb/22 ] | ||
|
We haven’t heard back from you for at least one calendar year, so this issue is being closed. If this is still an issue for you, please provide additional information and we will reopen the ticket. | ||
| Comment by Oleg Pudeyev (Inactive) [ 16/Apr/19 ] | ||
|
Okay changing this ticket to a feature request sounds good. | ||
| Comment by Gabriel Russell (Inactive) [ 16/Apr/19 ] | ||
The "data" parameter is not strictly required. failpoints can be written that do not need "data", and no failpoints, "failCommand" included, require "data" when "mode" is "off" so there can't be any sort of blanket "data" is always required validation. And if just merely altering the behavior of the "failCommand" failpoint is insufficient, the I suppose the only the only valuable solution would be to implement complete per-failpoint validation in configureFailpoint. Unless you disagree with this assessment, you or I should update this ticket to be a feature request for such an implementation. | ||
| Comment by Oleg Pudeyev (Inactive) [ 15/Apr/19 ] | ||
|
I think having proper validation of fail point parameters would be ideal, since from the drivers' perspective fail points are as much a part of the server's functionality as any of the "normal" commands. Drivers are increasing their use of fail points over time, thus providing diagnostics when fail points are not used correctly would save time for all driver engineers. With that, if the data parameter is stated to be required, perhaps simply adding validation to that effect would be sufficient for now? Even this simple change would help in the following cases in addition to the example given earlier:
I can see how altering the fail command fail point to always fail if, say, error code is not provided can create a different set of issues, therefore I think if additional validation is added it should be done when a fail point is set rather than when an operation is performed and the fail point is checked. | ||
| Comment by Gabriel Russell (Inactive) [ 15/Apr/19 ] | ||
|
Oleg, at first glance it looks to me that there is no failpoint specific validation that occurs in configureFailPoint . And there doesn't really seem to be must validation in any of the failpoint implementations I looked at just now. The few that I looked all all looked to be pretty flexible with their inputs. While the wiki does say that the data element is required, that doesn't seem to be enforced by code. Do you have any specific example of a failpoint validating its parameters? The data element of the configureFailPoint command is, in code and comments, explicitly optional. There may be existing failpoints that don't require data, and if there are not now, there could be in the future. I could alter the functionality of the 'failCommand" fail point so that it fails all commands when it doesn't have a data element or a contained failCommands element. I don't know if this is really useful ( otherwise someone probably would have done it already ) or worthwhile. Lastly we could heavily alter the implementation of configureFailPoint and of fail points in general and implement per fail point validation of configureFailPoint parameters. Which one of these ideas, if any, is closest to what you had in mind? |