Skip to content

Commit 1245eb9

Browse files
alcaeusalcaeus
committed
PHPLIB-466: Allow transactions to run on sharded clusters in MongoDB 4.2
1 parent 9d0cbbb commit 1245eb9

File tree

7 files changed

+172
-15
lines changed

7 files changed

+172
-15
lines changed

‎tests/FunctionalTestCase.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ protected function assertSameObjectId($expectedObjectId, $actualObjectId)
151151
* @param array|stdClass $command configureFailPoint command document
152152
* @throws InvalidArgumentException if $command is not a configureFailPoint command
153153
*/
154-
protected function configureFailPoint($command)
154+
public function configureFailPoint($command, Server $server = null)
155155
{
156156
if (! $this->isFailCommandSupported()) {
157157
$this->markTestSkipped('failCommand is only supported on mongod >= 4.0.0 and mongos >= 4.1.5.');
@@ -173,14 +173,16 @@ protected function configureFailPoint($command)
173173
throw new InvalidArgumentException('$command is not a configureFailPoint command');
174174
}
175175

176+
$failPointServer = $server ?: $this->getPrimaryServer();
177+
176178
$operation = new DatabaseCommand('admin', $command);
177-
$cursor = $operation->execute($this->getPrimaryServer());
179+
$cursor = $operation->execute($failPointServer);
178180
$result = $cursor->toArray()[0];
179181

180182
$this->assertCommandSucceeded($result);
181183

182184
// Record the fail point so it can be disabled during tearDown()
183-
$this->configuredFailPoints[] = $command->configureFailPoint;
185+
$this->configuredFailPoints[] = [$command->configureFailPoint, $failPointServer];
184186
}
185187

186188
/**
@@ -408,9 +410,7 @@ private function disableFailPoints()
408410
return;
409411
}
410412

411-
$server = $this->getPrimaryServer();
412-
413-
foreach ($this->configuredFailPoints as $failPoint) {
413+
foreach ($this->configuredFailPoints as list($failPoint, $server)) {
414414
$operation = new DatabaseCommand('admin', ['configureFailPoint' => $failPoint, 'mode' => 'off']);
415415
$operation->execute($server);
416416
}

‎tests/SpecTests/CommandExpectations.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use MongoDB\Driver\Monitoring\CommandSucceededEvent;
1111
use MultipleIterator;
1212
use function count;
13+
use function in_array;
1314
use function key;
1415
use function MongoDB\Driver\Monitoring\addSubscriber;
1516
use function MongoDB\Driver\Monitoring\removeSubscriber;
@@ -37,6 +38,9 @@ class CommandExpectations implements CommandSubscriber
3738
/** @var boolean */
3839
private $ignoreExtraEvents = false;
3940

41+
/** @var string[] */
42+
private $ignoredCommandNames = [];
43+
4044
private function __construct(array $events)
4145
{
4246
foreach ($events as $event) {
@@ -110,6 +114,14 @@ public static function fromTransactions(array $expectedEvents)
110114
$o->ignoreCommandFailed = true;
111115
$o->ignoreCommandSucceeded = true;
112116

117+
/* Ignore the buildInfo and getParameter commands as they are used to
118+
* check for the availability of configureFailPoint and are not expected
119+
* to be called by any spec tests.
120+
* configureFailPoint needs to be ignored as the targetedFailPoint
121+
* operation will be caught by command monitoring and is also not
122+
* present in the expected commands in spec tests. */
123+
$o->ignoredCommandNames = ['buildInfo', 'getParameter', 'configureFailPoint'];
124+
113125
return $o;
114126
}
115127

@@ -120,7 +132,7 @@ public static function fromTransactions(array $expectedEvents)
120132
*/
121133
public function commandFailed(CommandFailedEvent $event)
122134
{
123-
if ($this->ignoreCommandFailed || ($this->ignoreExtraEvents && count($this->actualEvents) === count($this->expectedEvents))) {
135+
if ($this->ignoreCommandFailed || $this->isEventIgnored($event)) {
124136
return;
125137
}
126138

@@ -134,7 +146,7 @@ public function commandFailed(CommandFailedEvent $event)
134146
*/
135147
public function commandStarted(CommandStartedEvent $event)
136148
{
137-
if ($this->ignoreCommandStarted || ($this->ignoreExtraEvents && count($this->actualEvents) === count($this->expectedEvents))) {
149+
if ($this->ignoreCommandStarted || $this->isEventIgnored($event)) {
138150
return;
139151
}
140152

@@ -148,7 +160,7 @@ public function commandStarted(CommandStartedEvent $event)
148160
*/
149161
public function commandSucceeded(CommandSucceededEvent $event)
150162
{
151-
if ($this->ignoreCommandSucceeded || ($this->ignoreExtraEvents && count($this->actualEvents) === count($this->expectedEvents))) {
163+
if ($this->ignoreCommandSucceeded || $this->isEventIgnored($event)) {
152164
return;
153165
}
154166

@@ -212,4 +224,10 @@ public function assert(FunctionalTestCase $test, Context $context)
212224
}
213225
}
214226
}
227+
228+
private function isEventIgnored($event)
229+
{
230+
return ($this->ignoreExtraEvents && count($this->actualEvents) === count($this->expectedEvents))
231+
|| in_array($event->getCommandName(), $this->ignoredCommandNames);
232+
}
215233
}

‎tests/SpecTests/Operation.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use MongoDB\Driver\Cursor;
1010
use MongoDB\Driver\Exception\BulkWriteException;
1111
use MongoDB\Driver\Exception\Exception;
12+
use MongoDB\Driver\Server;
1213
use MongoDB\Driver\Session;
1314
use MongoDB\Driver\WriteConcern;
1415
use MongoDB\GridFS\Bucket;
@@ -19,6 +20,7 @@
1920
use function array_map;
2021
use function fclose;
2122
use function fopen;
23+
use function in_array;
2224
use function MongoDB\is_last_pipeline_operator_write;
2325
use function MongoDB\with_transaction;
2426
use function stream_get_contents;
@@ -37,6 +39,7 @@ final class Operation
3739
const OBJECT_SELECT_DATABASE = 'selectDatabase';
3840
const OBJECT_SESSION0 = 'session0';
3941
const OBJECT_SESSION1 = 'session1';
42+
const OBJECT_TEST_RUNNER = 'testRunner';
4043

4144
/** @var ErrorExpectation|null */
4245
public $errorExpectation;
@@ -280,6 +283,8 @@ private function execute(FunctionalTestCase $test, Context $context)
280283
return $this->executeForSession($context->session0, $test, $context);
281284
case self::OBJECT_SESSION1:
282285
return $this->executeForSession($context->session1, $test, $context);
286+
case self::OBJECT_TEST_RUNNER:
287+
return $this->executeForTestRunner($test, $context);
283288
default:
284289
throw new LogicException('Unsupported object: ' . $this->object);
285290
}
@@ -545,6 +550,42 @@ private function executeForSession(Session $session, FunctionalTestCase $test, C
545550
}
546551
}
547552

553+
private function executeForTestRunner(FunctionalTestCase $test, Context $context)
554+
{
555+
$args = $context->prepareOptions($this->arguments);
556+
$context->replaceArgumentSessionPlaceholder($args);
557+
558+
switch ($this->name) {
559+
case 'assertSessionPinned':
560+
$test->assertInstanceOf(Session::class, $args['session']);
561+
$test->assertInstanceOf(Server::class, $args['session']->getServer());
562+
563+
return null;
564+
case 'assertSessionTransactionState':
565+
$test->assertInstanceOf(Session::class, $args['session']);
566+
/* PHPC currently does not expose the exact session state, but
567+
* instead exposes a bool to let us know whether a transaction
568+
* is currently in progress. This code may fail down the line
569+
* and should be adjusted once PHPC-1438 is implemented. */
570+
$expected = in_array($this->arguments['state'], ['in_progress', 'starting']);
571+
$test->assertSame($expected, $args['session']->isInTransaction());
572+
573+
return null;
574+
case 'assertSessionUnpinned':
575+
$test->assertInstanceOf(Session::class, $args['session']);
576+
$test->assertNull($args['session']->getServer());
577+
578+
return null;
579+
case 'targetedFailPoint':
580+
$test->assertInstanceOf(Session::class, $args['session']);
581+
$test->configureFailPoint($this->arguments['failPoint'], $args['session']->getServer());
582+
583+
return null;
584+
default:
585+
throw new LogicException('Unsupported test runner operation: ' . $this->name);
586+
}
587+
}
588+
548589
/**
549590
* @throws LogicException if the operation object is unsupported
550591
*/
@@ -561,6 +602,7 @@ private function getResultAssertionType()
561602
return ResultExpectation::ASSERT_SAME;
562603
case self::OBJECT_SESSION0:
563604
case self::OBJECT_SESSION1:
605+
case self::OBJECT_TEST_RUNNER:
564606
return ResultExpectation::ASSERT_NOTHING;
565607
default:
566608
throw new LogicException('Unsupported object: ' . $this->object);

‎tests/SpecTests/TransactionsSpecTest.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class TransactionsSpecTest extends FunctionalTestCase
3535
* @var array
3636
*/
3737
private static $incompleteTests = [
38+
'transactions/pin-mongos: remain pinned after non-transient error on commit' => 'Blocked on SPEC-1320',
3839
'transactions/read-pref: default readPreference' => 'PHPLIB does not properly inherit readPreference for transactions (PHPLIB-473)',
3940
'transactions/read-pref: primary readPreference' => 'PHPLIB does not properly inherit readPreference for transactions (PHPLIB-473)',
4041
'transactions/run-command: run command with secondary read preference in client option and primary read preference in transaction options' => 'PHPLIB does not properly inherit readPreference for transactions (PHPLIB-473)',
@@ -48,6 +49,8 @@ private function doSetUp()
4849
parent::setUp();
4950

5051
static::killAllSessions();
52+
53+
$this->skipIfTransactionsAreNotSupported();
5154
}
5255

5356
private function doTearDown()
@@ -128,8 +131,8 @@ public function testTransactions(stdClass $test, array $runOn = null, array $dat
128131
$this->markTestIncomplete(self::$incompleteTests[$this->dataDescription()]);
129132
}
130133

131-
if ($this->isShardedCluster()) {
132-
$this->markTestSkipped('PHP MongoDB driver 1.6.0alpha2 does not support running multi-document transactions on sharded clusters');
134+
if (isset($test->skipReason)) {
135+
$this->markTestSkipped($test->skipReason);
133136
}
134137

135138
if (isset($test->useMultipleMongoses) && $test->useMultipleMongoses && $this->isShardedCluster()) {

‎tests/SpecTests/transactions/errors-client.json

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
{
2+
"runOn": [
3+
{
4+
"minServerVersion": "4.0",
5+
"topology": [
6+
"replicaset"
7+
]
8+
},
9+
{
10+
"minServerVersion": "4.1.8",
11+
"topology": [
12+
"sharded"
13+
]
14+
}
15+
],
16+
"database_name": "transaction-tests",
17+
"collection_name": "test",
18+
"data": [],
19+
"tests": [
20+
{
21+
"description": "Client side error in command starting transaction",
22+
"operations": [
23+
{
24+
"name": "startTransaction",
25+
"object": "session0"
26+
},
27+
{
28+
"name": "insertOne",
29+
"object": "collection",
30+
"arguments": {
31+
"session": "session0",
32+
"document": {
33+
"_id": {
34+
".": "."
35+
}
36+
}
37+
},
38+
"error": true
39+
},
40+
{
41+
"name": "assertSessionTransactionState",
42+
"object": "testRunner",
43+
"arguments": {
44+
"session": "session0",
45+
"state": "starting"
46+
}
47+
}
48+
]
49+
},
50+
{
51+
"description": "Client side error when transaction is in progress",
52+
"operations": [
53+
{
54+
"name": "startTransaction",
55+
"object": "session0"
56+
},
57+
{
58+
"name": "insertOne",
59+
"object": "collection",
60+
"arguments": {
61+
"session": "session0",
62+
"document": {
63+
"_id": 4
64+
}
65+
},
66+
"result": {
67+
"insertedId": 4
68+
}
69+
},
70+
{
71+
"name": "insertOne",
72+
"object": "collection",
73+
"arguments": {
74+
"session": "session0",
75+
"document": {
76+
"_id": {
77+
".": "."
78+
}
79+
}
80+
},
81+
"error": true
82+
},
83+
{
84+
"name": "assertSessionTransactionState",
85+
"object": "testRunner",
86+
"arguments": {
87+
"session": "session0",
88+
"state": "in_progress"
89+
}
90+
}
91+
]
92+
}
93+
]
94+
}

‎tests/SpecTests/transactions/retryable-abort.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@
705705
}
706706
},
707707
{
708-
"description": "abortTransaction succeeds after InterruptedDueToStepDown",
708+
"description": "abortTransaction succeeds after InterruptedDueToReplStateChange",
709709
"failPoint": {
710710
"configureFailPoint": "failCommand",
711711
"mode": {
@@ -1627,7 +1627,7 @@
16271627
}
16281628
},
16291629
{
1630-
"description": "abortTransaction succeeds after WriteConcernError InterruptedDueToStepDown",
1630+
"description": "abortTransaction succeeds after WriteConcernError InterruptedDueToReplStateChange",
16311631
"failPoint": {
16321632
"configureFailPoint": "failCommand",
16331633
"mode": {

‎tests/SpecTests/transactions/retryable-commit.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@
942942
}
943943
},
944944
{
945-
"description": "commitTransaction succeeds after InterruptedDueToStepDown",
945+
"description": "commitTransaction succeeds after InterruptedDueToReplStateChange",
946946
"failPoint": {
947947
"configureFailPoint": "failCommand",
948948
"mode": {
@@ -1925,7 +1925,7 @@
19251925
}
19261926
},
19271927
{
1928-
"description": "commitTransaction succeeds after WriteConcernError InterruptedDueToStepDown",
1928+
"description": "commitTransaction succeeds after WriteConcernError InterruptedDueToReplStateChange",
19291929
"failPoint": {
19301930
"configureFailPoint": "failCommand",
19311931
"mode": {

0 commit comments

Comments
 (0)