[SERVER-34664] Commands that are unsupported in a transaction should error Created: 25/Apr/18  Updated: 29/Oct/23  Resolved: 27/Jul/18

Status: Closed
Project: Core Server
Component/s: Replication, Shell
Affects Version/s: 3.7.6
Fix Version/s: 4.0.2, 4.1.2

Type: Bug Priority: Major - P3
Reporter: Shane Harvey Assignee: Tess Avitabile (Inactive)
Resolution: Fixed Votes: 0
Labels: documentation-needed
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Related
is related to DOCS-11658 Document that certain commands do not... Closed
Backwards Compatibility: Minor Change
Operating System: ALL
Backport Requested:
v4.0
Sprint: Repl 2018-07-02, Repl 2018-07-16, Repl 2018-07-30
Participants:
Linked BF Score: 0

 Description   

This isMaster and ping commands (and possibly others) are not supported in a transaction but the server does not report an error when they are run within one.

> client = db.getMongo()
connection to 127.0.0.1:27017
> session = client.startSession()
> session.startTransaction()
> session.getDatabase("admin").runCommand('isMaster')
{
	"hosts" : "..."
}
> session.commitTransaction()
assert: command failed: {
	"operationTime" : Timestamp(1524616281, 1),
	"ok" : 0,
	"errmsg" : "Given transaction number 1 does not match any in-progress transactions.",
	"code" : 251,
	"codeName" : "NoSuchTransaction",
	"$clusterTime" : {
		"clusterTime" : Timestamp(1524616281, 1),
		"signature" : {
			"hash" : BinData(0,"AAAAAAAAAAAAAAAAAAAAAAAAAAA="),
			"keyId" : NumberLong(0)
		}
	}
}
_getErrorWithCode@src/mongo/shell/utils.js:25:13
doassert@src/mongo/shell/assert.js:16:14
_assertCommandWorked@src/mongo/shell/assert.js:510:17
assert.commandWorked@src/mongo/shell/assert.js:594:16
commitTransaction@src/mongo/shell/session.js:848:17
@(shell):1:1
 
2018-04-24T17:31:23.262-0700 E QUERY    [js] Error: command failed: {
	"operationTime" : Timestamp(1524616281, 1),
	"ok" : 0,
	"errmsg" : "Given transaction number 1 does not match any in-progress transactions.",
	"code" : 251,
	"codeName" : "NoSuchTransaction",
	"$clusterTime" : {
		"clusterTime" : Timestamp(1524616281, 1),
		"signature" : {
			"hash" : BinData(0,"AAAAAAAAAAAAAAAAAAAAAAAAAAA="),
			"keyId" : NumberLong(0)
		}
	}
} :
_getErrorWithCode@src/mongo/shell/utils.js:25:13
doassert@src/mongo/shell/assert.js:16:14
_assertCommandWorked@src/mongo/shell/assert.js:510:17
assert.commandWorked@src/mongo/shell/assert.js:594:16
commitTransaction@src/mongo/shell/session.js:848:17
@(shell):1:1

Expected behavior: the isMaster should error because it's not supported in a transaction. For example:

> session.startTransaction()
> session.getDatabase("admin").runCommand('isMaster')
{
	"operationTime" : Timestamp(1524616521, 1),
	"ok" : 0,
	"errmsg" : "Cannot run 'isMaster' in a multi-document transaction.",
	"code" : 50767,
	"codeName" : "Location50767",
	"$clusterTime" : {
		"clusterTime" : Timestamp(1524616521, 1),
		"signature" : {
			"hash" : BinData(0,"AAAAAAAAAAAAAAAAAAAAAAAAAAA="),
			"keyId" : NumberLong(0)
		}
	}
}



 Comments   
Comment by Githook User [ 06/Aug/18 ]

Author:

{'username': 'tessavitabile', 'name': 'Tess Avitabile', 'email': 'tess.avitabile@mongodb.com'}

Message: SERVER-34664 Commands that are unsupported in a transaction should error

(cherry picked from commit d56d941fe6d3d0af27ef8b8524e240e750a81364)
Branch: v4.0
https://github.com/mongodb/mongo/commit/135c6c1fa5de54a7b2e50c0163f361da90dd787e

Comment by Githook User [ 27/Jul/18 ]

Author:

{'username': 'tessavitabile', 'name': 'Tess Avitabile', 'email': 'tess.avitabile@mongodb.com'}

Message: SERVER-34664 Fix error code
Branch: master
https://github.com/mongodb/mongo/commit/782aeea024538b2d3a0cc6c18bcd59835ff3a2a4

Comment by Githook User [ 27/Jul/18 ]

Author:

{'username': 'tessavitabile', 'name': 'Tess Avitabile', 'email': 'tess.avitabile@mongodb.com'}

Message: SERVER-34664 Commands that are unsupported in a transaction should error
Branch: master
https://github.com/mongodb/mongo/commit/d56d941fe6d3d0af27ef8b8524e240e750a81364

Comment by Tess Avitabile (Inactive) [ 18/Jun/18 ]

This occurs because we do not parse the session/transaction command parameters for commands that do not require auth. Affected commands are:

  • _isSelf
  • authenticate
  • buildInfo
  • configureFailPoint
  • connectionStatus
  • echo
  • getLastError
  • getnonce
  • getPrevError
  • isMaster
  • listCommands
  • ping
  • refreshLogicalSessionCacheNow
  • resetError
  • saslContinue
  • saslStart
  • shutdown
  • whatsmyuri
    All other commands that are not allowed in transactions always return an error if used within a transaction. jesse, do you think it would be worthwhile to return an error if any of the above commands are used in transactions?
Comment by A. Jesse Jiryu Davis [ 06/Jun/18 ]

I wouldn't put much effort into fixing the behavior of "isMaster" in particular; drivers themselves don't call it in a transaction, and if user applications call it in a transaction, for some reason, then inconsistencies in its behavior are low-impact.

For commands in general, however, I agree that if a command is not fully supported in transactions then it should always error in a transaction.

Comment by Spencer Brody (Inactive) [ 05/Jun/18 ]

jesse shane.harvey, I'd like to re-open this discussion. We always intended to error if we receive a command with a txnNumber when the command isn't supported in transactions, but it looks like we didn't actually get that check working right. Leaving this as-is results in some weird behavioral inconsistencies. Namely some commands (such as isMaster) are okay to be sent within a transaction if there have already been ops in the transaction, but will cause an error if they are the first operation within the transaction. It'd be better for the behavior to be consistent, and I think the right consistent behavior is for them to fail. Is there any objection to the server making this change? jesse shane.harvey?

CC tess.avitabile

Comment by A. Jesse Jiryu Davis [ 25/Apr/18 ]

SGTM, thanks for thinking this through.

Comment by Shane Harvey [ 25/Apr/18 ]

The problem here is that the client thinks it started a transaction because it ran a command with all the necessary transaction parameters ('isMaster' in this case). When the client runs the commitTransaction command it finds out that the server never started a transaction. This is surprising.

That said I think I'm convinced that this doesn't need changes because banning 'isMaster' would be a little odd. As you said, they're not transactional. Perhaps we just need to clearly document the three types of commands:

  1. Commands that can be run in a transaction: insert, delete, ...
  2. Commands that can not be run in a transaction: listCollections, create, ...
  3. Commands that can be run in a transaction but have no affect: isMaster, ping, ...

jesse what do you think?

Comment by Eric Milkie [ 25/Apr/18 ]

One thing we could do here is make it an error in the shell if you attempt to use the commitTransaction() helper without having first done any transactional commands in that session after a startTransaction() call. If we wanted to test the behavior of the commitTransaction server command in that situation, we would simply have to run that command manually without the helper, in the test.

Comment by Kyle Suarez [ 25/Apr/18 ]

The drivers transactions spec doesn't mention that it's explicitly illegal to run those commands in a transaction, so I don't understand why we'd have to ban them. Like Eric said, they're not transactional; they're more like info commands.

Comment by Eric Milkie [ 25/Apr/18 ]

I think those commands are not transactional, so they will work just fine regardless of whether you have told the shell you want to start a new transaction in your session. Doesn't the shell give you that error for "commitTransaction()" because you didn't do any transactional operations beforehand? I feel like you would get the same error if you did your repro but left out the part where you ran "ismaster".

Generated at Thu Feb 08 04:37:26 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.