[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:
Related
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:

  • standalone and replica set deployments communicate the error via getLastError, but insert the document anyway
  • sharded cluster deployment communicate the error via getLastError and do not insert the document

Test code (Ruby):

require 'mongo'
 
def test(port)
  client = Mongo::Client.new(["localhost:#{port}"], server_api: {version: 1})
 
  client.use('test')['test'].delete_many
  p client.use('test')['test'].count
 
  client.cluster.next_primary.with_connection do |conn|
    insert = Mongo::Protocol::Insert.new('test', 'test', [{'name' => 'testing'}])
 
    p conn.dispatch([insert])
 
    query = Mongo::Protocol::Query.new('test', '$cmd', {getLastError: 1}, limit: 1)
 
    p conn.dispatch([query])
 
    query = Mongo::Protocol::Query.new('test', 'test', { 'name' => 'testing' })
 
    p conn.dispatch([query])
  end
 
  p client.use('test')['test'].count
  p client.use('test')['test'].find.first
end
 
[14900, 14920, 14940].each do |port|
  puts
  puts "Testing #{port}"
  test(port)
end

Result (14900 is standalone, 14920 is replica set, 14940 is sharded cluster):

Testing 14900
0
nil
#<Mongo::Protocol::Reply:0x000056461861e2c0 @flags=[:await_capable], @cursor_id=0, @starting_from=0, @number_returned=1, @documents=[{"ok"=>0.0, "errmsg"=>"The apiVersion parameter is required, please configure your MongoClient's API version", "code"=>498870, "codeName"=>"Location498870"}]>
#<Mongo::Protocol::Reply:0x000056461861c6f0 @flags=[:await_capable], @cursor_id=0, @starting_from=0, @number_returned=1, @documents=[{"_id"=>BSON::ObjectId('6053cac6baaddddc71400803'), "name"=>"testing"}], @upconverter=#<Mongo::Protocol::Reply::Upconverter:0x000056461861c1a0 @documents=[{"_id"=>BSON::ObjectId('6053cac6baaddddc71400803'), "name"=>"testing"}], @cursor_id=0, @starting_from=0>>
1
{"_id"=>BSON::ObjectId('6053cac6baaddddc71400803'), "name"=>"testing"}
 
Testing 14920
0
nil
#<Mongo::Protocol::Reply:0x00005646184a1640 @flags=[:await_capable], @cursor_id=0, @starting_from=0, @number_returned=1, @documents=[{"ok"=>0.0, "errmsg"=>"The apiVersion parameter is required, please configure your MongoClient's API version", "code"=>498870, "codeName"=>"Location498870", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00005646184a1258 @seconds=1616104134, @increment=2>, "signature"=>{"hash"=><BSON::Binary:0x1660 type=generic data=0x0000000000000000...>, "keyId"=>0}}, "operationTime"=>#<BSON::Timestamp:0x00005646184a0fb0 @seconds=1616104134, @increment=2>}]>
#<Mongo::Protocol::Reply:0x00005646184ae638 @flags=[:await_capable], @cursor_id=0, @starting_from=0, @number_returned=1, @documents=[{"_id"=>BSON::ObjectId('6053cac6bb72891ab2565f93'), "name"=>"testing"}], @upconverter=#<Mongo::Protocol::Reply::Upconverter:0x00005646184addf0 @documents=[{"_id"=>BSON::ObjectId('6053cac6bb72891ab2565f93'), "name"=>"testing"}], @cursor_id=0, @starting_from=0>>
1
{"_id"=>BSON::ObjectId('6053cac6bb72891ab2565f93'), "name"=>"testing"}
 
Testing 14940
0
nil
#<Mongo::Protocol::Reply:0x00005646183bfa88 @flags=[:await_capable], @cursor_id=0, @starting_from=0, @number_returned=1, @documents=[{"ok"=>0.0, "errmsg"=>"The apiVersion parameter is required, please configure your MongoClient's API version", "code"=>498870, "codeName"=>"Location498870", "$clusterTime"=>{"clusterTime"=>#<BSON::Timestamp:0x00005646183bf678 @seconds=1616104129, @increment=1>, "signature"=>{"hash"=><BSON::Binary:0x1780 type=generic data=0x0000000000000000...>, "keyId"=>0}}, "operationTime"=>#<BSON::Timestamp:0x00005646183bf268 @seconds=1616104129, @increment=1>}]>
#<Mongo::Protocol::Reply:0x00005646183bd648 @flags=[], @cursor_id=0, @starting_from=0, @number_returned=0, @documents=[], @upconverter=#<Mongo::Protocol::Reply::Upconverter:0x00005646183bd3c8 @documents=[], @cursor_id=0, @starting_from=0>>
0
nil



 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:

diff --git a/jstests/noPassthrough/require_api_version.js b/jstests/noPassthrough/require_api_version.js
index aafe763f3a..ea63b1c534 100644
--- a/jstests/noPassthrough/require_api_version.js
+++ b/jstests/noPassthrough/require_api_version.js
@@ -48,6 +48,22 @@ function runTest(db, supportsTransctions, writeConcern = {}, secondaries = []) {
     assert.commandWorked(
         db.runCommand({getMore: reply.cursor.id, collection: "collection", apiVersion: "1"}));
 
+    /*
+     * Legacy wire protocol is not supported.
+     */
+    const originalWriteMode = db.getMongo().writeMode();
+    db.getMongo().forceWriteMode("legacy");
+    assert.doesNotThrow(() => db["collection"].insert({_id: "OP_INSERT"}));
+    jsTestLog(db.getLastError());
+    db.getMongo().forceWriteMode(originalWriteMode);
+    // We expect that insert to have gone through and persisted.
+    /*
+    assert.commandFailedWithCode(
+        db.runCommand(
+            {insert: "collection", documents: [{_id: "OP_INSERT"}], apiVersion: "1", writeConcern}),
+        ErrorCodes.DuplicateKey);
+        */
+
     if (supportsTransctions) {
         /*
          * Commands in transactions require API version.
diff --git a/src/mongo/db/initialize_api_parameters.cpp b/src/mongo/db/initialize_api_parameters.cpp
index 812a452ea3..6b9d7d6f2a 100644
--- a/src/mongo/db/initialize_api_parameters.cpp
+++ b/src/mongo/db/initialize_api_parameters.cpp
@@ -108,7 +108,7 @@ void enforceRequireAPIVersion(OperationContext* opCtx, Command* command) {
         !client->session() || (client->session()->getTags() & transport::Session::kInternalClient);
 
     if (gRequireApiVersion.load() && !opCtx->getClient()->isInDirectClient() && !isInternalClient) {
-        uassert(
+        tassert(
             498870,
             "The apiVersion parameter is required, please configure your MongoClient's API version",
             APIParameters::get(opCtx).getParamsPassed());

Generated at Thu Feb 08 05:36:07 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.