-
Notifications
You must be signed in to change notification settings - Fork 208
PHPC-1152, PHPC-1161: Emulate implicit sessions for command cursors #803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
aec79b2
Refactor command and query execute functions
jmikola 427854e
PHPC-1152: Create implicit session for commands
jmikola fdc7565
PHPC-1161: Free reference to Session once Cursor is exhausted
jmikola 1f222d0
PHPC-1151: Bump Cursor var_dump array size for Session
jmikola File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
--TEST-- | ||
PHPC-1152: Command cursors should use the same session for getMore and killCursors (implicit) | ||
--SKIPIF-- | ||
<?php if (PHP_INT_SIZE !== 8) { die("skip Can't represent 64-bit ints on a 32-bit platform"); } ?> | ||
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?> | ||
<?php NEEDS_CRYPTO(); ?> | ||
<?php NEEDS('STANDALONE'); ?> | ||
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?> | ||
<?php CLEANUP(STANDALONE); ?> | ||
--FILE-- | ||
<?php | ||
require_once __DIR__ . "/../utils/basic.inc"; | ||
|
||
class Test implements MongoDB\Driver\Monitoring\CommandSubscriber | ||
{ | ||
private $lsidByCursorId = []; | ||
private $lsidByRequestId = []; | ||
|
||
public function executeCommand() | ||
{ | ||
$manager = new MongoDB\Driver\Manager(STANDALONE); | ||
|
||
$bulk = new MongoDB\Driver\BulkWrite; | ||
$bulk->insert(['_id' => 1]); | ||
$bulk->insert(['_id' => 2]); | ||
$bulk->insert(['_id' => 3]); | ||
$manager->executeBulkWrite(NS, $bulk); | ||
|
||
$command = new MongoDB\Driver\Command([ | ||
'aggregate' => COLLECTION_NAME, | ||
'pipeline' => [['$match' => new stdClass]], | ||
'cursor' => ['batchSize' => 2], | ||
]); | ||
|
||
MongoDB\Driver\Monitoring\addSubscriber($this); | ||
|
||
/* By creating two cursors with the same name, PHP's reference counting | ||
* will destroy the first after the second is created. Note that | ||
* mongoc_cursor_destroy also destroys implicit sessions and returns | ||
* them to the LIFO pool. This sequencing allows us to test that getMore | ||
* and killCursors use the session ID corresponding to the original | ||
* aggregate command. */ | ||
$cursor = $manager->executeCommand(DATABASE_NAME, $command); | ||
$cursor->toArray(); | ||
|
||
$cursor = $manager->executeCommand(DATABASE_NAME, $command); | ||
$cursor->toArray(); | ||
|
||
$cursor = $manager->executeCommand(DATABASE_NAME, $command); | ||
$cursor = $manager->executeCommand(DATABASE_NAME, $command); | ||
unset($cursor); | ||
|
||
MongoDB\Driver\Monitoring\removeSubscriber($this); | ||
|
||
/* We should expect two unique session IDs over the course of the test, | ||
* since at most two implicit sessions would have been in use at any | ||
* given time. */ | ||
printf("Unique session IDs used: %d\n", count(array_unique($this->lsidByRequestId))); | ||
} | ||
|
||
public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event) | ||
{ | ||
$requestId = $event->getRequestId(); | ||
$sessionId = bin2hex((string) $event->getCommand()->lsid->id); | ||
|
||
printf("%s session ID: %s\n", $event->getCommandName(), $sessionId); | ||
|
||
if ($event->getCommandName() === 'aggregate') { | ||
if (isset($this->lsidByRequestId[$requestId])) { | ||
throw new UnexpectedValueException('Previous command observed for request ID: ' . $requestId); | ||
} | ||
|
||
$this->lsidByRequestId[$requestId] = $sessionId; | ||
} | ||
|
||
if ($event->getCommandName() === 'getMore') { | ||
$cursorId = $event->getCommand()->getMore; | ||
|
||
if ( ! isset($this->lsidByCursorId[$cursorId])) { | ||
throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId); | ||
} | ||
|
||
printf("getMore used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no'); | ||
} | ||
|
||
if ($event->getCommandName() === 'killCursors') { | ||
$cursorId = $event->getCommand()->cursors[0]; | ||
|
||
if ( ! isset($this->lsidByCursorId[$cursorId])) { | ||
throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId); | ||
} | ||
|
||
printf("killCursors used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no'); | ||
} | ||
} | ||
|
||
public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event) | ||
{ | ||
/* Associate the aggregate's session ID with its cursor ID so it can be | ||
* looked up by the subsequent getMore or killCursors */ | ||
if ($event->getCommandName() === 'aggregate') { | ||
$cursorId = $event->getReply()->cursor->id; | ||
$requestId = $event->getRequestId(); | ||
|
||
$this->lsidByCursorId[$cursorId] = $this->lsidByRequestId[$requestId]; | ||
} | ||
} | ||
|
||
public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event) | ||
{ | ||
} | ||
} | ||
|
||
(new Test)->executeCommand(); | ||
|
||
?> | ||
===DONE=== | ||
<?php exit(0); ?> | ||
--EXPECTF-- | ||
aggregate session ID: %x | ||
getMore session ID: %x | ||
getMore used same session as aggregate: yes | ||
aggregate session ID: %x | ||
getMore session ID: %x | ||
getMore used same session as aggregate: yes | ||
aggregate session ID: %x | ||
aggregate session ID: %x | ||
killCursors session ID: %x | ||
killCursors used same session as aggregate: yes | ||
killCursors session ID: %x | ||
killCursors used same session as aggregate: yes | ||
Unique session IDs used: 2 | ||
===DONE=== |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before you merge this into master, you should run
clang-format
on this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in the v1.4 branch, which isn't formatted. I intend to run it through clang-format after (or during) the merge to master. Does that work?