Skip to content
This repository was archived by the owner on Feb 7, 2024. It is now read-only.

[fix] Redis connection counter didn't work properly #497

Merged
merged 26 commits into from
Sep 6, 2020

Conversation

rennokki
Copy link
Collaborator

@rennokki rennokki commented Sep 3, 2020

For the Redis replicator, each time a new connection is opened or closed, it will increment a value in the database.

Until now, the connection counts were kept in the channel manager and it caused the multi-node deployments to fail to display the right amount of connections and even track them (for example, keeping track the max amount of connections for each app) because the connection counts were hold into each node's memory.

The Redis database will keep a value for the opened connections for each app, instead of letting each server declare the connections number for each app.

@rennokki rennokki changed the base branch from master to 2.x September 3, 2020 13:42
@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2020

Codecov Report

Merging #497 into 2.x will decrease coverage by 16.11%.
The diff coverage is 15.72%.

Impacted file tree graph

@@              Coverage Diff              @@
##                2.x     #497       +/-   ##
=============================================
- Coverage     78.13%   62.01%   -16.12%     
- Complexity        0      355      +355     
=============================================
  Files            55       56        +1     
  Lines          1372     1477      +105     
=============================================
- Hits           1072      916      -156     
- Misses          300      561      +261     
Impacted Files Coverage Δ Complexity Δ
src/PubSub/Drivers/RedisClient.php 0.00% <0.00%> (-67.28%) 31.00 <5.00> (+31.00) ⬇️
src/Statistics/Logger/RedisStatisticsLogger.php 0.00% <0.00%> (-64.78%) 17.00 <0.00> (+17.00) ⬇️
...s/Channels/ChannelManagers/RedisChannelManager.php 0.00% <0.00%> (ø) 2.00 <2.00> (?)
src/PubSub/Drivers/LocalClient.php 86.48% <50.00%> (-10.07%) 15.00 <4.00> (+15.00) ⬇️
src/WebSockets/WebSocketHandler.php 82.05% <52.63%> (-9.89%) 22.00 <4.00> (+22.00) ⬇️
src/Statistics/Logger/MemoryStatisticsLogger.php 91.89% <100.00%> (ø) 12.00 <0.00> (+12.00)
...s/Channels/ChannelManagers/ArrayChannelManager.php 97.29% <100.00%> (+0.15%) 13.00 <2.00> (+13.00)
src/WebSocketsServiceProvider.php 98.03% <100.00%> (+0.08%) 4.00 <0.00> (+4.00)
src/Console/RestartWebSocketServer.php 80.00% <0.00%> (-20.00%) 1.00% <0.00%> (+1.00%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9b85bb...d20adcd. Read the comment docs.

@rennokki rennokki mentioned this pull request Sep 3, 2020
14 tasks
@rennokki
Copy link
Collaborator Author

rennokki commented Sep 5, 2020

Just managed to turn the then()s into blocking I/O for the unit testing 🎉 The PR got testing for functionality (with 2 incomplete tests tho; might look after it soon).

@rennokki rennokki merged commit c078e5a into 2.x Sep 6, 2020
@rennokki rennokki deleted the fix/app-connections-count branch September 6, 2020 08:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants