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.

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

Stop tracking build user counts when shutting down #206

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Dec 18, 2023

As per https://discord.com/channels/188630481301012481/188630652340404224/1186195069062094918

Simplest feasible solution.

I also considered making BuildCountUserUpdater implement IEntityStore but it's a stretch. It doesn't really track entities, so two out of three members of the interface don't make sense for the class. You could split the one method that does to an interface, but at that point the split-out interface would become a cheap copy of IDisposable anyways.

Can be tested via something like the following (probably won't apply directly due to the REPLAYS_PATH I have set locally):

diff --git a/osu.Server.Spectator/GracefulShutdownManager.cs b/osu.Server.Spectator/GracefulShutdownManager.cs
index f75323c..8a06c98 100644
--- a/osu.Server.Spectator/GracefulShutdownManager.cs
+++ b/osu.Server.Spectator/GracefulShutdownManager.cs
@@ -25,7 +25,7 @@ namespace osu.Server.Spectator
         // This should probably be configurable in the future.
         // 6 hours is way too long, but set initially to test the whole process out.
         // We can manually override this for immediate shutdown if/when required from a kubernetes or docker level.
-        public static readonly TimeSpan TIME_BEFORE_FORCEFUL_SHUTDOWN = TimeSpan.FromHours(6);
+        public static readonly TimeSpan TIME_BEFORE_FORCEFUL_SHUTDOWN = TimeSpan.FromSeconds(30);
 
         private readonly List<IEntityStore> dependentStores = new List<IEntityStore>();
         private readonly EntityStore<ServerMultiplayerRoom> roomStore;
@@ -82,8 +82,8 @@ namespace osu.Server.Spectator
             {
                 var remaining = dependentStores.Select(store => (store.EntityName, store.RemainingUsages));
 
-                if (remaining.Sum(s => s.RemainingUsages) == 0)
-                    break;
+                //if (remaining.Sum(s => s.RemainingUsages) == 0)
+                //    break;
 
                 Logger.Log("Waiting for usages of existing entities to finish...");
                 foreach (var r in remaining)
diff --git a/osu.Server.Spectator/Hubs/Metadata/BuildUserCountUpdater.cs b/osu.Server.Spectator/Hubs/Metadata/BuildUserCountUpdater.cs
index 55d072d..ebe2ee7 100644
--- a/osu.Server.Spectator/Hubs/Metadata/BuildUserCountUpdater.cs
+++ b/osu.Server.Spectator/Hubs/Metadata/BuildUserCountUpdater.cs
@@ -20,7 +20,7 @@ namespace osu.Server.Spectator.Hubs.Metadata
         /// <summary>
         /// Amount of time (in milliseconds) between subsequent updates of user counts.
         /// </summary>
-        public int UpdateInterval = 300_000;
+        public int UpdateInterval = 1000;
 
         private readonly EntityStore<MetadataClientState> clientStates;
         private readonly IDatabaseFactory databaseFactory;
diff --git a/osu.Server.Spectator/Properties/launchSettings.json b/osu.Server.Spectator/Properties/launchSettings.json
index c3ea1ca..1a3c792 100644
--- a/osu.Server.Spectator/Properties/launchSettings.json
+++ b/osu.Server.Spectator/Properties/launchSettings.json
@@ -17,7 +17,8 @@
       "environmentVariables": {
         "ASPNETCORE_ENVIRONMENT": "Development",
         "SAVE_REPLAYS": "1",
-        "REPLAYS_PATH": "/home/dachb/Documents/opensource/osu-web/public/uploads-solo-replay"
+        "REPLAYS_PATH": "/home/dachb/Documents/opensource/osu-web/public/uploads-solo-replay",
+        "TRACK_BUILD_USER_COUNTS": "1"
       }
     }
   }

@peppy peppy merged commit 49d3940 into ppy:master Dec 18, 2023
2 checks passed
@bdach bdach deleted the stop-tracking-during-shutdown branch December 18, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants