[SERVER-55318] Standalone and replica set servers insert documents using OP_INSERT when API version is required and not provided Created: 18/Mar/21 Updated: 24/Jun/21 Resolved: 24/Jun/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | 4.9.0-alpha4 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Oleg Pudeyev (Inactive) | Assignee: | A. Jesse Jiryu Davis |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Operating System: | ALL | ||||
| Sprint: | Query Optimization 2021-05-03, Query Optimization 2021-05-17, Query Optimization 2021-05-31, Query Optimization 2021-06-14, Query Optimization 2021-06-28 | ||||
| Participants: | |||||
| Description |
|
When I use OP_INSERT to talk to 4.9.0-alpha5 server, and I have the server configured to require API version, and I do not provide an API version in the OP_INSERT:
Test code (Ruby):
Result (14900 is standalone, 14920 is replica set, 14940 is sharded cluster):
|
| Comments |
| Comment by Charlie Swanson [ 24/Jun/21 ] | ||||||||||||||||||||||||||||||||||||||||
|
Thank you both! | ||||||||||||||||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 24/Jun/21 ] | ||||||||||||||||||||||||||||||||||||||||
|
Then I think it's best to close this ticket and move on to other things. New drivers that implement the Versioned API won't use the legacy messages like OP_INSERT at all, so I don't expect this inconsistency to have a big impact. When OP_INSERT and other obsolete messages are removed from the server, then this bug (if it is a bug) will go away. | ||||||||||||||||||||||||||||||||||||||||
| Comment by Oleg Pudeyev (Inactive) [ 24/Jun/21 ] | ||||||||||||||||||||||||||||||||||||||||
|
I suppose if api version is required, a regular driver will always provide it, assuming the driver is correctly implemented. So perhaps the most likely casualty of this bug is a driver developer who sees weird behavior. | ||||||||||||||||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 24/Jun/21 ] | ||||||||||||||||||||||||||||||||||||||||
|
I've assigned this to myself. oleg.pudeyev is it important that mongod and mongos treat OP_INSERT the same when requireApiVersion is enabled? Since OP_INSERT is deprecated and requireApiVersion is test-only, this seems to be a very small intersection of use cases. What's the impact if we leave the code as-is? | ||||||||||||||||||||||||||||||||||||||||
| Comment by Oleg Pudeyev (Inactive) [ 23/Jun/21 ] | ||||||||||||||||||||||||||||||||||||||||
|
The result that the server reports (error inserting) doesn't match actual outcome (document is inserted). This seems important to me but perhaps you disagree? The Ruby driver hasn't needed to work around this bug to my knowledge so far, I believe this is something I ran into while performing manual testing. | ||||||||||||||||||||||||||||||||||||||||
| Comment by Charlie Swanson [ 23/Jun/21 ] | ||||||||||||||||||||||||||||||||||||||||
|
I have investigated this enough to determine the difference in behavior can be attributed to the fact that mongos receives an OP_INSERT request but sends a different request to the shards in the form of a normal command. This command of course does not include any api version, so the shard will reject it with an error before performing any insert. I am not totally sure about the getLastError behavior you're seeing. If the insert is indeed performed then it seems to me it's possible that the 'getLastError' command itself is the one generating the error about the missing api version? jesse is it OK if I hand this off to you? Looks like much of the code in the area was authored by you. oleg.pudeyev my understanding is that the 'require api version' parameter is only being used for testing on the drivers side now - so is this ticket of particular importance anymore? I have the following patch applied locally to attempt to reproduce on our end:
|