Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add possibility to use locking with DynamoDB sessions #1676

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add possibility to use locking with DynamoDB sessions #1676

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
781163e
Add possibility to use locking with DynamoDB sessions
valtzu Mar 3, 2024
4e8182e
Remove explicit type casting
valtzu Mar 3, 2024
a5ef2a6
Don't give up before timeout is reached
valtzu Mar 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Integration/Aws/DynamoDbSession/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Added

- Added support for `async-aws/dynamo-db` 2.0 and 3.0
- Support locking by configuring `locking`, `max_lock_wait_time`, `min_lock_retry_microtime` & `max_lock_retry_microtime`

## 1.0.2

Expand Down
2 changes: 1 addition & 1 deletion src/Integration/Aws/DynamoDbSession/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
],
"require": {
"php": "^7.2.5 || ^8.0",
"async-aws/dynamo-db": "^1.0 || ^2.0 || ^3.0"
"async-aws/dynamo-db": "^1.1 || ^2.0 || ^3.0"
},
"autoload": {
"psr-4": {
Expand Down
73 changes: 65 additions & 8 deletions src/Integration/Aws/DynamoDbSession/src/SessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use AsyncAws\DynamoDb\Enum\BillingMode;
use AsyncAws\DynamoDb\Enum\KeyType;
use AsyncAws\DynamoDb\Enum\ScalarAttributeType;
use AsyncAws\DynamoDb\Exception\ConditionalCheckFailedException;
use AsyncAws\DynamoDb\ValueObject\AttributeValue;

class SessionHandler implements \SessionHandlerInterface
{
Expand All @@ -17,13 +19,17 @@ class SessionHandler implements \SessionHandlerInterface

/**
* @var array{
* consistent_read?: bool,
* consistent_read: bool,
* data_attribute: string,
* hash_key: string,
* session_lifetime?: int,
* session_lifetime_attribute: string,
* table_name: string,
* id_separator: string
* id_separator: string,
* locking: bool,
* max_lock_wait_time: float,
* min_lock_retry_microtime: int<0, max>,
* max_lock_retry_microtime: int<0, max>,
* }
*/
private $options;
Expand Down Expand Up @@ -56,7 +62,11 @@ class SessionHandler implements \SessionHandlerInterface
* session_lifetime?: int,
* session_lifetime_attribute?: string,
* table_name: string,
* id_separator?: string
* id_separator?: string,
* locking?: bool,
* max_lock_wait_time?: int|float,
* min_lock_retry_microtime?: int<0, max>,
* max_lock_retry_microtime?: int<0, max>,
* } $options
*/
public function __construct(DynamoDbClient $client, array $options)
Expand All @@ -66,6 +76,11 @@ public function __construct(DynamoDbClient $client, array $options)
$options['hash_key'] = $options['hash_key'] ?? 'id';
$options['session_lifetime_attribute'] = $options['session_lifetime_attribute'] ?? 'expires';
$options['id_separator'] = $options['id_separator'] ?? '_';
$options['consistent_read'] = $options['consistent_read'] ?? true;
$options['locking'] = (bool) ($options['locking'] ?? false);
jderusse marked this conversation as resolved.
Show resolved Hide resolved
$options['max_lock_wait_time'] = (float) ($options['max_lock_wait_time'] ?? 10.0);
$options['min_lock_retry_microtime'] = (int) ($options['min_lock_retry_microtime'] ?? 10000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure such thing deserve parameters.

Copy link
Author

@valtzu valtzu Mar 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand DynamoDB usually is billed per-request, so giving a possibility to fine-tune these according to your use case may be useful in terms of balancing the functionality & cost-savings. For example, if your sessions usually are locking for several seconds, you likely don't want to be polling every 10-50ms (which again is the default from official sdk).

Also, these settings are just copied from aws-sdk to make a migration to async-aws as small change as possible.

$options['max_lock_retry_microtime'] = (int) ($options['max_lock_retry_microtime'] ?? 50000);
$this->options = $options;
}

Expand Down Expand Up @@ -166,11 +181,8 @@ public function read($sessionId)

$this->dataRead = '';

$attributes = $this->client->getItem([
'TableName' => $this->options['table_name'],
'Key' => $this->formatKey($this->formatId($sessionId)),
'ConsistentRead' => $this->options['consistent_read'] ?? true,
])->getItem();
$key = $this->formatKey($this->formatId($sessionId));
$attributes = $this->options['locking'] ? $this->readLocked($key) : $this->readNonLocked($key);

// Return the data if it is not expired. If it is expired, remove it
if (isset($attributes[$this->options['session_lifetime_attribute']]) && isset($attributes[$this->options['data_attribute']])) {
Expand Down Expand Up @@ -207,6 +219,10 @@ private function doWrite(string $id, bool $updateData, string $data = ''): bool
$this->options['session_lifetime_attribute'] => ['Value' => ['N' => (string) $expires]],
];

if ($this->options['locking']) {
$attributes['lock'] = ['Action' => 'DELETE'];
}

if ($updateData) {
$attributes[$this->options['data_attribute']] = '' != $data
? ['Value' => ['S' => $data]]
Expand Down Expand Up @@ -238,4 +254,45 @@ private function formatKey(string $key): array
{
return [$this->options['hash_key'] => ['S' => $key]];
}

/**
* @return array<string, AttributeValue>
*/
private function readNonLocked(array $key): array
{
return $this->client->getItem([
'TableName' => $this->options['table_name'],
'Key' => $key,
'ConsistentRead' => $this->options['consistent_read'],
])->getItem();
}

/**
* @return array<string, AttributeValue>
*/
private function readLocked(array $key): array
{
$timeout = microtime(true) + $this->options['max_lock_wait_time'];

while (true) {
try {
return $this->client->updateItem([
'TableName' => $this->options['table_name'],
'Key' => $key,
'ConsistentRead' => $this->options['consistent_read'],
'Expected' => ['lock' => ['Exists' => false]],
'AttributeUpdates' => ['lock' => ['Value' => ['N' => '1']]],
'ReturnValues' => 'ALL_NEW',
])->getAttributes();
} catch (ConditionalCheckFailedException $e) {
// If we were to exceed the timeout after sleep, let's give up immediately.
$sleep = rand($this->options['min_lock_retry_microtime'], $this->options['max_lock_retry_microtime']);
jderusse marked this conversation as resolved.
Show resolved Hide resolved
if (microtime(true) + $sleep * 1e-6 > $timeout) {
jderusse marked this conversation as resolved.
Show resolved Hide resolved
throw $e;
}

usleep($sleep);
}
}
}
}
215 changes: 215 additions & 0 deletions src/Integration/Aws/DynamoDbSession/tests/SessionHandlerLockingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
<?php

namespace AsyncAws\DynamoDbSession\Tests;

use AsyncAws\Core\AwsError\AwsError;
use AsyncAws\Core\Test\ResultMockFactory;
use AsyncAws\DynamoDb\DynamoDbClient;
use AsyncAws\DynamoDb\Exception\ConditionalCheckFailedException;
use AsyncAws\DynamoDb\Result\UpdateItemOutput;
use AsyncAws\DynamoDb\ValueObject\AttributeValue;
use AsyncAws\DynamoDbSession\SessionHandler;
use PHPUnit\Framework\TestCase;
use Symfony\Contracts\HttpClient\ResponseInterface;

class SessionHandlerLockingTest extends TestCase
{
/**
* @var \PHPUnit\Framework\MockObject\MockObject|DynamoDbClient
*/
private $client;

/**
* @var SessionHandler
*/
private $handler;

protected function setUp(): void
{
$this->client = $this->createMock(DynamoDbClient::class);

$this->handler = new SessionHandler($this->client, [
'table_name' => 'testTable',
'session_lifetime' => 86400,
'locking' => true,
'max_lock_wait_time' => 1.0,
'min_lock_retry_microtime' => 300000,
'max_lock_retry_microtime' => 300000,
]);

$this->handler->open(null, 'PHPSESSID');
}

public function testRead(): void
{
$this->client
->expects(self::once())
->method('updateItem')
->with(self::equalTo([
'TableName' => 'testTable',
'Key' => [
'id' => [
'S' => 'PHPSESSID_123456789',
],
],
'ConsistentRead' => true,
'Expected' => ['lock' => ['Exists' => false]],
'AttributeUpdates' => ['lock' => ['Value' => ['N' => '1']]],
'ReturnValues' => 'ALL_NEW',
]))
->willReturn(ResultMockFactory::create(UpdateItemOutput::class, [
'Attributes' => [
'data' => new AttributeValue(['S' => 'test data']),
'expires' => new AttributeValue(['N' => (string) (time() + 86400)]),
],
]));

self::assertEquals('test data', $this->handler->read('123456789'));
}

public static function readLockProvider(): array
{
return [
'success' => [3, 2, true],
'timeout' => [4, 4, false],
];
}

/**
* @dataProvider readLockProvider
*/
public function testReadLock(int $attempts, int $failCount, bool $expectedSuccess): void
{
$this->client
->expects($matcher = self::exactly($attempts))
->method('updateItem')
->with(self::equalTo([
'TableName' => 'testTable',
'Key' => [
'id' => [
'S' => 'PHPSESSID_123456789',
],
],
'ConsistentRead' => true,
'Expected' => ['lock' => ['Exists' => false]],
'AttributeUpdates' => ['lock' => ['Value' => ['N' => '1']]],
'ReturnValues' => 'ALL_NEW',
]))
->willReturnCallback(function () use ($matcher, $failCount) {
if ($matcher->getInvocationCount() <= $failCount) {
$mockResponse = self::createMock(ResponseInterface::class);
$mockResponse->method('getInfo')->willReturnMap([['http_code', 400]]);

throw new ConditionalCheckFailedException($mockResponse, new AwsError('a', 'b', 'c', 'd'));
}

return ResultMockFactory::create(UpdateItemOutput::class, [
'Attributes' => [
'data' => new AttributeValue(['S' => 'test data']),
'expires' => new AttributeValue(['N' => (string) (time() + 86400)]),
],
]);
});

if (!$expectedSuccess) {
self::expectException(ConditionalCheckFailedException::class);
}

self::assertEquals('test data', $this->handler->read('123456789'));
}

public function testWriteWithUnchangedData(): void
{
$this->client
->method('updateItem')
->willReturnMap([
[
[
'TableName' => 'testTable',
'Key' => ['id' => ['S' => 'PHPSESSID_123456789']],
'ConsistentRead' => true,
'Expected' => ['lock' => ['Exists' => false]],
'AttributeUpdates' => ['lock' => ['Value' => ['N' => '1']]],
'ReturnValues' => 'ALL_NEW',
],
ResultMockFactory::create(UpdateItemOutput::class, [
'Attributes' => [
'data' => new AttributeValue(['S' => 'previous data']),
'expires' => new AttributeValue(['N' => (string) (time() + 86400)]),
],
]),
],
[
[
'TableName' => 'testTable',
'Key' => [
'id' => ['S' => 'PHPSESSID_123456789'],
],
'AttributeUpdates' => [
'expires' => ['Value' => ['N' => (string) (time() + 86400)]],
'lock' => ['Action' => 'DELETE'],
],
],
ResultMockFactory::create(UpdateItemOutput::class, [
'Attributes' => [
'data' => new AttributeValue(['S' => 'previous data']),
'expires' => new AttributeValue(['N' => (string) (time() + 86400)]),
],
]),
],
]);

$this->handler->read('123456789');

self::assertTrue($this->handler->write('123456789', 'previous data'));
}

public function testWriteWithChangedData(): void
{
$this->client
->method('updateItem')
->willReturnMap([
[
[
'TableName' => 'testTable',
'Key' => ['id' => ['S' => 'PHPSESSID_123456789']],
'ConsistentRead' => true,
'Expected' => ['lock' => ['Exists' => false]],
'AttributeUpdates' => [
'lock' => ['Value' => ['N' => '1']],
],
'ReturnValues' => 'ALL_NEW',
],
ResultMockFactory::create(UpdateItemOutput::class, [
'Attributes' => [
'data' => new AttributeValue(['S' => 'previous data']),
'expires' => new AttributeValue(['N' => (string) (time() + 86400)]),
],
]),
],
[
[
'TableName' => 'testTable',
'Key' => [
'id' => ['S' => 'PHPSESSID_123456789'],
],
'AttributeUpdates' => [
'expires' => ['Value' => ['N' => (string) (time() + 86400)]],
'lock' => ['Action' => 'DELETE'],
'data' => ['Value' => ['S' => 'new data']],
],
],
ResultMockFactory::create(UpdateItemOutput::class, [
'Attributes' => [
'data' => new AttributeValue(['S' => 'previous data']),
'expires' => new AttributeValue(['N' => (string) (time() + 86400)]),
],
]),
],
]);

$this->handler->read('123456789');

self::assertTrue($this->handler->write('123456789', 'new data'));
}
}
Loading