From 8d1369ee0248879196cfb81d29c8441ffa9aaf7b Mon Sep 17 00:00:00 2001 From: Alex Renoki Date: Mon, 7 Dec 2020 23:15:09 +0200 Subject: [PATCH 1/5] Fixed peak connections count not being able to settle down --- src/Statistics/Collectors/RedisCollector.php | 2 +- src/Statistics/Statistic.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Statistics/Collectors/RedisCollector.php b/src/Statistics/Collectors/RedisCollector.php index 050eb74..a7bd00f 100644 --- a/src/Statistics/Collectors/RedisCollector.php +++ b/src/Statistics/Collectors/RedisCollector.php @@ -249,7 +249,7 @@ class RedisCollector extends MemoryCollector $this->channelManager->getPublishClient()->hset( $this->channelManager->getRedisKey($appId, null, ['stats']), - 'peak_connections_count', $currentConnectionCount + 'peak_connections_count', max(0, $currentConnectionCount) ); $this->channelManager->getPublishClient()->hset( diff --git a/src/Statistics/Statistic.php b/src/Statistics/Statistic.php index 1a92488..5e1f05f 100644 --- a/src/Statistics/Statistic.php +++ b/src/Statistics/Statistic.php @@ -178,7 +178,7 @@ class Statistic public function reset(int $currentConnectionsCount) { $this->currentConnectionsCount = $currentConnectionsCount; - $this->peakConnectionsCount = $currentConnectionsCount; + $this->peakConnectionsCount = max(0, $currentConnectionsCount); $this->webSocketMessagesCount = 0; $this->apiMessagesCount = 0; } From 9a0d56d6d3c76b4350aa25c95cc15529b46a1990 Mon Sep 17 00:00:00 2001 From: Alex Renoki Date: Mon, 7 Dec 2020 23:16:51 +0200 Subject: [PATCH 2/5] Reset app traces if no activity was found since last save --- src/Contracts/StatisticsCollector.php | 9 +++++++++ src/Statistics/Collectors/MemoryCollector.php | 18 ++++++++++++++++++ src/Statistics/Collectors/RedisCollector.php | 6 ++++++ src/Statistics/Statistic.php | 12 ++++++++++++ 4 files changed, 45 insertions(+) diff --git a/src/Contracts/StatisticsCollector.php b/src/Contracts/StatisticsCollector.php index a46e757..316a83b 100644 --- a/src/Contracts/StatisticsCollector.php +++ b/src/Contracts/StatisticsCollector.php @@ -66,4 +66,13 @@ interface StatisticsCollector * @return PromiseInterface[\BeyondCode\LaravelWebSockets\Statistics\Statistic|null] */ public function getAppStatistics($appId): PromiseInterface; + + /** + * Remove all app traces from the database if no connections have been set + * in the meanwhile since last save. + * + * @param string|int $appId + * @return void + */ + public function resetAppTraces($appId); } diff --git a/src/Statistics/Collectors/MemoryCollector.php b/src/Statistics/Collectors/MemoryCollector.php index 2394e0a..80070dc 100644 --- a/src/Statistics/Collectors/MemoryCollector.php +++ b/src/Statistics/Collectors/MemoryCollector.php @@ -92,6 +92,12 @@ class MemoryCollector implements StatisticsCollector continue; } + if ($statistic->shouldHaveTracesRemoved()) { + $this->resetAppTraces($appId); + + continue; + } + $this->createRecord($statistic, $appId); $this->channelManager->getGlobalConnectionsCount($appId)->then(function ($connections) use ($statistic) { @@ -136,6 +142,18 @@ class MemoryCollector implements StatisticsCollector ); } + /** + * Remove all app traces from the database if no connections have been set + * in the meanwhile since last save. + * + * @param string|int $appId + * @return void + */ + public function resetAppTraces($appId) + { + unset($this->statistics[$appId]); + } + /** * Find or create a defined statistic for an app. * diff --git a/src/Statistics/Collectors/RedisCollector.php b/src/Statistics/Collectors/RedisCollector.php index a7bd00f..e487a94 100644 --- a/src/Statistics/Collectors/RedisCollector.php +++ b/src/Statistics/Collectors/RedisCollector.php @@ -161,6 +161,10 @@ class RedisCollector extends MemoryCollector $appId, Helpers::redisListToArray($list) ); + if ($statistic->shouldHaveTracesRemoved()) { + return $this->resetAppTraces($appId); + } + $this->createRecord($statistic, $appId); $this->channelManager @@ -272,6 +276,8 @@ class RedisCollector extends MemoryCollector */ public function resetAppTraces($appId) { + parent::resetAppTraces($appId); + $this->channelManager->getPublishClient()->hdel( $this->channelManager->getRedisKey($appId, null, ['stats']), 'current_connections_count' diff --git a/src/Statistics/Statistic.php b/src/Statistics/Statistic.php index 5e1f05f..b31d547 100644 --- a/src/Statistics/Statistic.php +++ b/src/Statistics/Statistic.php @@ -183,6 +183,18 @@ class Statistic $this->apiMessagesCount = 0; } + /** + * Check if the current statistic entry is empty. This means + * that the statistic entry can be easily deleted if no activity + * occured for a while. + * + * @return bool + */ + public function shouldHaveTracesRemoved(): bool + { + return $this->currentConnectionsCount === 0 && $this->peakConnectionsCount === 0; + } + /** * Transform the statistic to array. * From c7aea38cdc6214286e2b3e230276870767174c75 Mon Sep 17 00:00:00 2001 From: Alex Renoki Date: Mon, 7 Dec 2020 23:19:11 +0200 Subject: [PATCH 3/5] Testing --- tests/PresenceChannelTest.php | 12 ++++++++ tests/StatisticsStoreTest.php | 57 +++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/tests/PresenceChannelTest.php b/tests/PresenceChannelTest.php index f1427af..1b8ae5a 100644 --- a/tests/PresenceChannelTest.php +++ b/tests/PresenceChannelTest.php @@ -117,6 +117,18 @@ class PresenceChannelTest extends TestCase $this->channelManager->getChannelMembers('1234', 'presence-channel')->then(function ($members) { $this->assertCount(2, $members); }); + + $this->pusherServer->onClose($rick); + $this->pusherServer->onClose($morty); + $this->pusherServer->onClose($pickleRick); + + $this->channelManager->getGlobalConnectionsCount('1234', 'presence-channel')->then(function ($total) { + $this->assertEquals(3, $total); + }); + + $this->channelManager->getChannelMembers('1234', 'presence-channel')->then(function ($members) { + $this->assertCount(2, $members); + }); } public function test_presence_channel_broadcast_member_events() diff --git a/tests/StatisticsStoreTest.php b/tests/StatisticsStoreTest.php index 6fe6cc2..b0b22be 100644 --- a/tests/StatisticsStoreTest.php +++ b/tests/StatisticsStoreTest.php @@ -16,6 +16,23 @@ class StatisticsStoreTest extends TestCase $this->assertEquals('2', $records[0]['peak_connections_count']); $this->assertEquals('2', $records[0]['websocket_messages_count']); $this->assertEquals('0', $records[0]['api_messages_count']); + + $this->pusherServer->onClose($rick); + $this->pusherServer->onClose($morty); + + $this->statisticsCollector->save(); + + $this->assertCount(2, $records = $this->statisticsStore->getRecords()); + + $this->assertEquals('2', $records[1]['peak_connections_count']); + $this->assertEquals('0', $records[1]['websocket_messages_count']); + $this->assertEquals('0', $records[1]['api_messages_count']); + + $this->statisticsCollector->save(); + + // The last one should not generate any more records + // since the current state is empty. + $this->assertCount(2, $records = $this->statisticsStore->getRecords()); } public function test_store_statistics_on_private_channel() @@ -30,19 +47,55 @@ class StatisticsStoreTest extends TestCase $this->assertEquals('2', $records[0]['peak_connections_count']); $this->assertEquals('2', $records[0]['websocket_messages_count']); $this->assertEquals('0', $records[0]['api_messages_count']); + + $this->pusherServer->onClose($rick); + $this->pusherServer->onClose($morty); + + $this->statisticsCollector->save(); + + $this->assertCount(2, $records = $this->statisticsStore->getRecords()); + + $this->assertEquals('2', $records[1]['peak_connections_count']); + $this->assertEquals('0', $records[1]['websocket_messages_count']); + $this->assertEquals('0', $records[1]['api_messages_count']); + + $this->statisticsCollector->save(); + + // The last one should not generate any more records + // since the current state is empty. + $this->assertCount(2, $records = $this->statisticsStore->getRecords()); } public function test_store_statistics_on_presence_channel() { $rick = $this->newPresenceConnection('presence-channel', ['user_id' => 1]); $morty = $this->newPresenceConnection('presence-channel', ['user_id' => 2]); + $pickleRick = $this->newPresenceConnection('presence-channel', ['user_id' => 1]); $this->statisticsCollector->save(); $this->assertCount(1, $records = $this->statisticsStore->getRecords()); - $this->assertEquals('2', $records[0]['peak_connections_count']); - $this->assertEquals('2', $records[0]['websocket_messages_count']); + $this->assertEquals('3', $records[0]['peak_connections_count']); + $this->assertEquals('3', $records[0]['websocket_messages_count']); $this->assertEquals('0', $records[0]['api_messages_count']); + + $this->pusherServer->onClose($rick); + $this->pusherServer->onClose($morty); + $this->pusherServer->onClose($pickleRick); + + $this->statisticsCollector->save(); + + $this->assertCount(2, $records = $this->statisticsStore->getRecords()); + + $this->assertEquals('3', $records[1]['peak_connections_count']); + $this->assertEquals('0', $records[1]['websocket_messages_count']); + $this->assertEquals('0', $records[1]['api_messages_count']); + + $this->statisticsCollector->save(); + + // The last one should not generate any more records + // since the current state is empty. + $this->assertCount(2, $records = $this->statisticsStore->getRecords()); } } From 2d30edb4f63d89016a3e60be4d55d9325e8c1e3f Mon Sep 17 00:00:00 2001 From: Alex Renoki Date: Mon, 7 Dec 2020 23:29:28 +0200 Subject: [PATCH 4/5] Reverted test --- tests/PresenceChannelTest.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/PresenceChannelTest.php b/tests/PresenceChannelTest.php index 1b8ae5a..f1427af 100644 --- a/tests/PresenceChannelTest.php +++ b/tests/PresenceChannelTest.php @@ -117,18 +117,6 @@ class PresenceChannelTest extends TestCase $this->channelManager->getChannelMembers('1234', 'presence-channel')->then(function ($members) { $this->assertCount(2, $members); }); - - $this->pusherServer->onClose($rick); - $this->pusherServer->onClose($morty); - $this->pusherServer->onClose($pickleRick); - - $this->channelManager->getGlobalConnectionsCount('1234', 'presence-channel')->then(function ($total) { - $this->assertEquals(3, $total); - }); - - $this->channelManager->getChannelMembers('1234', 'presence-channel')->then(function ($members) { - $this->assertCount(2, $members); - }); } public function test_presence_channel_broadcast_member_events() From a99b5d00043af7ae50f2bf95152bfa81bddf3d3c Mon Sep 17 00:00:00 2001 From: Alex Renoki Date: Mon, 7 Dec 2020 23:35:18 +0200 Subject: [PATCH 5/5] Reverted check for messages count --- tests/StatisticsStoreTest.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/StatisticsStoreTest.php b/tests/StatisticsStoreTest.php index b0b22be..419341b 100644 --- a/tests/StatisticsStoreTest.php +++ b/tests/StatisticsStoreTest.php @@ -25,8 +25,6 @@ class StatisticsStoreTest extends TestCase $this->assertCount(2, $records = $this->statisticsStore->getRecords()); $this->assertEquals('2', $records[1]['peak_connections_count']); - $this->assertEquals('0', $records[1]['websocket_messages_count']); - $this->assertEquals('0', $records[1]['api_messages_count']); $this->statisticsCollector->save(); @@ -56,8 +54,6 @@ class StatisticsStoreTest extends TestCase $this->assertCount(2, $records = $this->statisticsStore->getRecords()); $this->assertEquals('2', $records[1]['peak_connections_count']); - $this->assertEquals('0', $records[1]['websocket_messages_count']); - $this->assertEquals('0', $records[1]['api_messages_count']); $this->statisticsCollector->save(); @@ -89,8 +85,6 @@ class StatisticsStoreTest extends TestCase $this->assertCount(2, $records = $this->statisticsStore->getRecords()); $this->assertEquals('3', $records[1]['peak_connections_count']); - $this->assertEquals('0', $records[1]['websocket_messages_count']); - $this->assertEquals('0', $records[1]['api_messages_count']); $this->statisticsCollector->save();