From 77080261840ce98ea3808f570a7ef72ebeb08b78 Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Sun, 6 Jan 2019 16:00:21 -0500 Subject: SERVER-38717 Shell prohibits w: 0 in session --- .../suites/retryable_writes_jscore_passthrough.yml | 3 + .../suites/session_jscore_passthrough.yml | 4 + jstests/core/batch_write_command_delete.js | 63 --------- jstests/core/batch_write_command_insert.js | 15 --- jstests/core/batch_write_command_update.js | 52 ------- jstests/core/batch_write_command_w0.js | 149 +++++++++++++++++++++ jstests/noPassthrough/implicit_sessions.js | 5 + jstests/noPassthrough/session_w0.js | 20 +++ src/mongo/shell/session.js | 32 +++-- 9 files changed, 202 insertions(+), 141 deletions(-) create mode 100644 jstests/core/batch_write_command_w0.js create mode 100644 jstests/noPassthrough/session_w0.js diff --git a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml index 96635ea9da7..040049f458a 100644 --- a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_passthrough.yml @@ -44,6 +44,9 @@ selector: # the retryable writes suite overrides the runCommand to repeat commands. - jstests/core/failcommand_failpoint.js + # Unacknowledged writes prohibited in an explicit session. + - jstests/core/batch_write_command_w0.js + executor: archive: hooks: diff --git a/buildscripts/resmokeconfig/suites/session_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/session_jscore_passthrough.yml index f279dd70a90..4c709b1d53b 100644 --- a/buildscripts/resmokeconfig/suites/session_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/session_jscore_passthrough.yml @@ -12,6 +12,10 @@ selector: - jstests/core/max_time_ms.js - jstests/core/validate_cmd_ns.js + # Unacknowledged writes prohibited in an explicit session. + - jstests/core/crud_api.js + - jstests/core/batch_write_command_w0.js + executor: archive: hooks: diff --git a/jstests/core/batch_write_command_delete.js b/jstests/core/batch_write_command_delete.js index bb87fa4876d..48234dbeef5 100644 --- a/jstests/core/batch_write_command_delete.js +++ b/jstests/core/batch_write_command_delete.js @@ -69,22 +69,6 @@ assert(resultOK(result), tojson(result)); assert.eq(1, result.n); assert.eq(0, coll.count()); -// -// Single document delete, w:0 write concern specified -coll.remove({}); -coll.insert({a: 1}); -request = { - delete: coll.getName(), - deletes: [{q: {a: 1}, limit: 1}], - writeConcern: {w: 0} -}; -result = coll.runCommand(request); -assert(resultOK(result), tojson(result)); -countEventually(coll, 0); - -var fields = ['ok']; -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - // // Single document remove, w:1 write concern specified, ordered:true coll.remove({}); @@ -215,50 +199,3 @@ assert.eq(1, result.writeErrors[1].index); assert.eq('number', typeof result.writeErrors[1].code); assert.eq('string', typeof result.writeErrors[1].errmsg); assert.eq(0, coll.count()); - -// -// Cause remove error using ordered:false and w:0 -coll.remove({}); -coll.insert({a: 1}); -request = { - delete: coll.getName(), - deletes: [{q: {$set: {a: 1}}, limit: 0}, {q: {$set: {a: 1}}, limit: 0}, {q: {a: 1}, limit: 0}], - writeConcern: {w: 0}, - ordered: false -}; -result = coll.runCommand(request); -assert.commandWorked(result); -countEventually(coll, 0); - -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - -// -// Cause remove error using ordered:true and w:0 -coll.remove({}); -coll.insert({a: 1}); -request = { - delete: coll.getName(), - deletes: - [{q: {$set: {a: 1}}, limit: 0}, {q: {$set: {a: 1}}, limit: 0}, {q: {a: 1}, limit: (1)}], - writeConcern: {w: 0}, - ordered: true -}; -result = coll.runCommand(request); -assert.commandWorked(result); -countEventually(coll, 1); - -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - -// -// When limit is not 0 and 1 -coll.remove({}); -coll.insert({a: 1}); -request = { - delete: coll.getName(), - deletes: [{q: {a: 1}, limit: 2}], - writeConcern: {w: 0}, - ordered: false -}; -result = coll.runCommand(request); -// Unacknowledged writes are always OK -assert.commandWorked(result); diff --git a/jstests/core/batch_write_command_insert.js b/jstests/core/batch_write_command_insert.js index feddcb50ea6..dcbe065bf19 100644 --- a/jstests/core/batch_write_command_insert.js +++ b/jstests/core/batch_write_command_insert.js @@ -69,21 +69,6 @@ assert(resultOK(result), tojson(result)); assert.eq(1, result.n); assert.eq(coll.count(), 1); -// -// Single document insert, w:0 write concern specified, missing ordered -coll.remove({}); -request = { - insert: coll.getName(), - documents: [{a: 1}], - writeConcern: {w: 0} -}; -result = coll.runCommand(request); -assert(resultOK(result), tojson(result)); -countEventually(coll, 1); - -var fields = ['ok']; -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - // // Single document insert, w:1 write concern specified, ordered:true coll.remove({}); diff --git a/jstests/core/batch_write_command_update.js b/jstests/core/batch_write_command_update.js index 1c124bfadb7..df1a0ade62a 100644 --- a/jstests/core/batch_write_command_update.js +++ b/jstests/core/batch_write_command_update.js @@ -113,40 +113,6 @@ upsertedId = result.upserted[0]._id; assert.eq(1, coll.count({_id: upsertedId})); assert.eq(0, result.nModified, "missing/wrong nModified"); -// -// Single document upsert, write concern 0 specified, ordered = true -coll.remove({}); -request = { - update: coll.getName(), - updates: [{q: {a: 1}, u: {$set: {a: 1}}, upsert: true}], - writeConcern: {w: 0}, - ordered: true -}; -result = coll.runCommand(request); -assert(resultOK(result), tojson(result)); -countEventually(coll, 1); - -var fields = ['ok']; -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - -// -// Two document upsert, write concern 0 specified, ordered = true -coll.remove({}); -request = { - update: coll.getName(), - updates: [ - {q: {a: 2}, u: {$set: {a: 1}}, upsert: true}, - {q: {a: 2}, u: {$set: {a: 2}}, upsert: true} - ], - writeConcern: {w: 0}, - ordered: true -}; -result = coll.runCommand(request); -assert(resultOK(result), tojson(result)); -countEventually(coll, 2); - -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - // // Single document update coll.remove({}); @@ -268,24 +234,6 @@ assert.eq(1, coll.count()); coll.remove({}); coll.ensureIndex({a: 1}, {unique: true}); -// -// Upsert fail due to duplicate key index, w:0, ordered:true -coll.remove({}); -request = { - update: coll.getName(), - updates: [ - {q: {b: 1}, u: {$set: {b: 1, a: 1}}, upsert: true}, - {q: {b: 2}, u: {$set: {b: 2, a: 1}}, upsert: true} - ], - writeConcern: {w: 0}, - ordered: true -}; -result = coll.runCommand(request); -assert(result.ok, tojson(result)); -countEventually(coll, 1); - -assert.hasFields(result, fields, 'fields in result do not match: ' + tojson(fields)); - // // Upsert fail due to duplicate key index, w:1, ordered:true coll.remove({}); diff --git a/jstests/core/batch_write_command_w0.js b/jstests/core/batch_write_command_w0.js new file mode 100644 index 00000000000..387b576d4b9 --- /dev/null +++ b/jstests/core/batch_write_command_w0.js @@ -0,0 +1,149 @@ +/** + * Test unacknowledged write commands. + * + * Cannot implicitly shard accessed collections because of following errmsg: A single + * update/delete on a sharded collection must contain an exact match on _id or contain the shard + * key. + * + * @tags: [ + * assumes_unsharded_collection, + * assumes_write_concern_unchanged, + * requires_non_retryable_writes, + * ] + */ + +function countEventually(collection, n) { + assert.soon( + function() { + return collection.count() === n; + }, + function() { + return "unacknowledged write timed out"; + }); +} + +var coll = db.getCollection("batch_write_w0"); +coll.drop(); + +// +// Ensures that mongod respects the batch write protocols for delete +// +assert(coll.getDB().getMongo().useWriteCommands(), "test is not running with write commands"); + +// EACH TEST BELOW SHOULD BE SELF-CONTAINED, FOR EASIER DEBUGGING + +// +// Single document insert, w:0 write concern specified, missing ordered +coll.remove({}); +request = { + insert: coll.getName(), + documents: [{a: 1}], + writeConcern: {w: 0} +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +countEventually(coll, 1); + +// +// Single document upsert, write concern 0 specified, ordered = true +coll.remove({}); +request = { + update: coll.getName(), + updates: [{q: {a: 1}, u: {$set: {a: 1}}, upsert: true}], + writeConcern: {w: 0}, + ordered: true +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +countEventually(coll, 1); + +// +// Two document upsert, write concern 0 specified, ordered = true +coll.remove({}); +request = { + update: coll.getName(), + updates: [ + {q: {a: 2}, u: {$set: {a: 1}}, upsert: true}, + {q: {a: 2}, u: {$set: {a: 2}}, upsert: true} + ], + writeConcern: {w: 0}, + ordered: true +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +countEventually(coll, 2); + +// +// Upsert fail due to duplicate key index, w:0, ordered:true +coll.remove({}); +coll.ensureIndex({a: 1}, {unique: true}); +request = { + update: coll.getName(), + updates: [ + {q: {b: 1}, u: {$set: {b: 1, a: 1}}, upsert: true}, + {q: {b: 2}, u: {$set: {b: 2, a: 1}}, upsert: true} + ], + writeConcern: {w: 0}, + ordered: true +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +countEventually(coll, 1); + +// Remove unique index +coll.drop(); + +// +// Single document delete, w:0 write concern specified +coll.remove({}); +coll.insert({a: 1}); +request = { + delete: coll.getName(), + deletes: [{q: {a: 1}, limit: 1}], + writeConcern: {w: 0} +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +countEventually(coll, 0); + +// +// Cause remove error using ordered:false and w:0 +coll.remove({}); +coll.insert({a: 1}); +request = { + delete: coll.getName(), + deletes: [{q: {$set: {a: 1}}, limit: 0}, {q: {$set: {a: 1}}, limit: 0}, {q: {a: 1}, limit: 0}], + writeConcern: {w: 0}, + ordered: false +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +countEventually(coll, 0); + +// +// Cause remove error using ordered:true and w:0 - $set isn't a valid delete filter +coll.remove({}); +coll.insert({a: 1}); +request = { + delete: coll.getName(), + deletes: [{q: {$set: {a: 1}}, limit: 0}, {q: {$set: {a: 1}}, limit: 0}, {q: {a: 1}, limit: 1}], + writeConcern: {w: 0}, + ordered: true +}; +result = coll.runCommand(request); +assert.eq({ok: 1}, result); +assert.eq(coll.count(), 1); + +// +// When limit is not 0 and 1 +coll.remove({}); +coll.insert({a: 1}); +request = { + delete: coll.getName(), + deletes: [{q: {a: 1}, limit: 2}], + writeConcern: {w: 0}, + ordered: false +}; +result = coll.runCommand(request); +// Unacknowledged writes are always OK +assert.eq({ok: 1}, result); diff --git a/jstests/noPassthrough/implicit_sessions.js b/jstests/noPassthrough/implicit_sessions.js index 887da75101f..77204e098e9 100644 --- a/jstests/noPassthrough/implicit_sessions.js +++ b/jstests/noPassthrough/implicit_sessions.js @@ -73,6 +73,11 @@ assert.writeOK(coll.insert({x: 1})); }, {shouldIncludeId: true}); + // Unacknowledged writes have no session id. + inspectCommandForSessionId(function() { + coll.insert({x: 1}, {writeConcern: {w: 0}}); + }, {shouldIncludeId: false}); + assert(bsonBinaryEqual(testDB.getSession().getSessionId(), implicitId), "Expected the id of the database's implicit session to match the one sent, sent: " + tojson(implicitId) + " db session id: " + diff --git a/jstests/noPassthrough/session_w0.js b/jstests/noPassthrough/session_w0.js new file mode 100644 index 00000000000..dd219581f43 --- /dev/null +++ b/jstests/noPassthrough/session_w0.js @@ -0,0 +1,20 @@ +/** + * Explicit shell session should prohibit w: 0 writes. + */ +(function() { + "use strict"; + + const conn = MongoRunner.runMongod(); + const session = conn.startSession(); + const sessionColl = session.getDatabase("test").getCollection("foo"); + const err = assert.throws(() => { + sessionColl.insert({x: 1}, {writeConcern: {w: 0}}); + }); + + assert.includes(err.toString(), + "Unacknowledged writes are prohibited with sessions", + "wrong error message"); + + session.endSession(); + MongoRunner.stopMongod(conn); +})(); diff --git a/src/mongo/shell/session.js b/src/mongo/shell/session.js index d1587440545..62d5a070dfa 100644 --- a/src/mongo/shell/session.js +++ b/src/mongo/shell/session.js @@ -16,6 +16,18 @@ var { return typeof obj === "object" && obj !== null; } + function isAcknowledged(cmdObj) { + if (isNonNullObject(cmdObj.writeConcern)) { + const writeConcern = cmdObj.writeConcern; + // Intentional use of "==" comparing NumberInt, NumberLong, or plain Number. + if (writeConcern.hasOwnProperty("w") && writeConcern.w == 0) { + return false; + } + } + + return true; + } + function SessionOptions(rawOptions = {}) { if (!(this instanceof SessionOptions)) { return new SessionOptions(rawOptions); @@ -219,6 +231,10 @@ var { } function prepareCommandRequest(driverSession, cmdObj) { + if (driverSession._isExplicit && !isAcknowledged(cmdObj)) { + throw new Error("Unacknowledged writes are prohibited with sessions"); + } + if (serverSupports(kWireVersionSupportingLogicalSession) && // Always attach sessionId from explicit sessions. (driverSession._isExplicit || @@ -549,7 +565,9 @@ var { } if (!cmdObjUnwrapped.hasOwnProperty("lsid")) { - cmdObjUnwrapped.lsid = this.handle.getId(); + if (isAcknowledged(cmdObjUnwrapped)) { + cmdObjUnwrapped.lsid = this.handle.getId(); + } // We consider the session to still be in use by the client any time the session id // is injected into the command object as part of making a request. @@ -594,16 +612,8 @@ var { return false; } - if (isNonNullObject(cmdObj.writeConcern)) { - const writeConcern = cmdObj.writeConcern; - - // We use bsonWoCompare() in order to handle cases where the "w" field is specified - // as a NumberInt() or NumberLong() instance. - if (writeConcern.hasOwnProperty("w") && - bsonWoCompare({_: writeConcern.w}, {_: 0}) === 0) { - // Unacknowledged writes cannot be retried. - return false; - } + if (!isAcknowledged(cmdObj)) { + return false; } if (cmdName === "insert") { -- cgit v1.2.1