From 19eec5a745d56f3f0e9803aff57e5002fb9910bf Mon Sep 17 00:00:00 2001 From: gladysheva Date: Mon, 12 Feb 2024 12:48:49 +0500 Subject: [PATCH] Don't remove app from cache if env xists --- .../ApplicationsStorage_Tests.cs | 99 ++++++++++++++++--- .../EnvironmentsStorage_Tests.cs | 29 ++++-- Vostok.ServiceDiscovery/ServiceLocator.cs | 2 +- .../ApplicationWithReplicas.cs | 4 +- .../ApplicationsStorage.cs | 8 +- .../EnvironmentsStorage.cs | 3 +- 6 files changed, 120 insertions(+), 25 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index 305dbee..4bac018 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -4,7 +4,6 @@ using NUnit.Framework; using Vostok.Commons.Testing; using Vostok.ServiceDiscovery.Abstractions.Models; -using Vostok.ServiceDiscovery.Models; using Vostok.ServiceDiscovery.ServiceLocatorStorage; using Vostok.ZooKeeper.Client.Abstractions; @@ -20,7 +19,7 @@ public void Should_track_application_properties() CreateApplicationNode("default", "application"); CreateReplicaNode(new ReplicaInfo("default", "application", "https://github.com/vostok")); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { for (var times = 0; times < 10; times++) { @@ -46,7 +45,7 @@ public void Should_track_application_replicas() CreateEnvironmentNode("default"); CreateApplicationNode("default", "application"); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { var expectedReplicas = new List(); @@ -69,7 +68,7 @@ public void Should_return_null_without_application() { CreateEnvironmentNode("default"); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { ShouldReturnImmediately(storage, "default", "application", null); @@ -85,7 +84,7 @@ public void Should_return_empty_list_without_replicas() CreateEnvironmentNode("default"); CreateApplicationNode("default", "application"); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { ShouldReturnImmediately(storage, "default", "application", ServiceTopology.Build(new Uri[0], null)); @@ -106,7 +105,7 @@ public void Should_store_multiple_environments_and_applications() CreateApplicationNode("environment2", "application1", new Dictionary {{"key", "2/1"}}); CreateApplicationNode("environment2", "application2", new Dictionary {{"key", "2/2"}}); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { ShouldReturnImmediately( storage, @@ -134,7 +133,7 @@ public void Should_works_disconnected() CreateApplicationNode("default", "application"); CreateReplicaNode(new ReplicaInfo("default", "application", "https://github.com/vostok")); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { var properties = new Dictionary { @@ -162,7 +161,7 @@ public void Should_not_update_after_dispose() CreateApplicationNode("default", "application", new Dictionary {{"key", "value1"}}); CreateReplicaNode(new ReplicaInfo("default", "application", "https://github.com/vostok")); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { var expected = ServiceTopology.Build(new List {new Uri("https://github.com/vostok")}, new Dictionary {{"key", "value1"}}); ShouldReturnImmediately(storage, "default", "application", expected); @@ -182,7 +181,7 @@ public void Should_not_update_to_invalid_application_properties() CreateApplicationNode("default", "application", new Dictionary {{"key", "value"}}); CreateReplicaNode(new ReplicaInfo("default", "application", "https://github.com/vostok")); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { var expected = ServiceTopology.Build(new List {new Uri("https://github.com/vostok")}, new Dictionary {{"key", "value"}}); ShouldReturnImmediately(storage, "default", "application", expected); @@ -201,7 +200,7 @@ public void UpdateAll_should_force_update() CreateEnvironmentNode("default"); CreateApplicationNode("default", "application"); - using (var storage = GetApplicationsStorage()) + using (var storage = GetApplicationsStorage(out _)) { var expectedReplicas = new List(); @@ -224,6 +223,81 @@ public void UpdateAll_should_force_update() } } + [Test] + public void Should_delete_application_from_cache_if_app_and_env_nodes_were_deleted_when_observation_of_deleted_apps_is_disabled() + { + CreateEnvironmentNode("environment1"); + CreateApplicationNode("environment1", "application1", new Dictionary {{"key", "1/1"}}); + + using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) + { + var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); + ShouldReturnImmediately( + storage, + "environment1", + "application1", + expectedTopology); + + DeleteApplicationNode("environment1", "application1"); + DeleteEnvironmentNode("environment1"); + + envStorage.UpdateAll(); + storage.UpdateAll(); + + storage.Contains("environment1", "application1").Should().BeFalse(); + } + } + + [Test] + public void Should_not_delete_application_from_cache_when_env_exists_and_observation_of_deleted_apps_is_disabled() + { + CreateEnvironmentNode("environment1"); + CreateApplicationNode("environment1", "application1", new Dictionary {{"key", "1/1"}}); + + using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) + { + var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); + ShouldReturnImmediately( + storage, + "environment1", + "application1", + expectedTopology); + + DeleteApplicationNode("environment1", "application1"); + + envStorage.UpdateAll(); + envStorage.Contains("environment1").Should().BeTrue(); + storage.UpdateAll(); + + storage.Contains("environment1", "application1").Should().BeTrue(); + } + } + + [Test] + public void Should_not_delete_application_from_cache_when_observation_of_deleted_apps_is_disabled_and_client_disconnected() + { + CreateEnvironmentNode("environment1"); + CreateApplicationNode("environment1", "application1", new Dictionary {{"key", "1/1"}}); + + using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) + { + var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); + ShouldReturnImmediately( + storage, + "environment1", + "application1", + expectedTopology); + + Ensemble.Stop(); + + envStorage.UpdateAll(); + envStorage.Contains("environment1").Should().BeTrue(); + storage.UpdateAll(); + + storage.Contains("environment1", "application1").Should().BeTrue(); + } + } + private static void ShouldReturn(ApplicationsStorage storage, string environment, string application, ServiceTopology topology) { Action assertion = () => { ShouldReturnImmediately(storage, environment, application, topology); }; @@ -235,9 +309,10 @@ private static void ShouldReturnImmediately(ApplicationsStorage storage, string storage.Get(environment, application).ServiceTopology.Should().BeEquivalentTo(topology); } - private ApplicationsStorage GetApplicationsStorage() + private ApplicationsStorage GetApplicationsStorage(out EnvironmentsStorage envStorage, bool observeNonExistentApplications = true) { - return new ApplicationsStorage(ZooKeeperClient, PathHelper, EventsQueue, true, Log); + envStorage = new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, observeNonExistentApplications, Log); + return new ApplicationsStorage(ZooKeeperClient, PathHelper, EventsQueue, observeNonExistentApplications, envStorage, Log); } } } \ No newline at end of file diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs index a049c37..7eb247e 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -4,7 +4,6 @@ using NUnit.Framework; using Vostok.Commons.Testing; using Vostok.ServiceDiscovery.Abstractions.Models; -using Vostok.ServiceDiscovery.Models; using Vostok.ServiceDiscovery.ServiceLocatorStorage; using Vostok.ZooKeeper.Client.Abstractions; @@ -167,14 +166,32 @@ public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_ob DeleteEnvironmentNode("default"); storage.UpdateAll(); - storage.EnvironmentsInStorage.Should().Be(1); + storage.Contains("default").Should().BeTrue(); storage.Get("default").Should().BeNull(); - + CreateEnvironmentNode("default", "parent"); - storage.Get("default").Should().BeEquivalentTo(expectedInfo); + ShouldReturn(storage, "default", expectedInfo); } } - + + [Test] + public void Should_not_delete_environment_from_cache_when_observation_of_deleted_apps_is_disabled_and_client_disconnected() + { + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: true)) + { + CreateEnvironmentNode("default", "parent"); + + var expectedInfo = new EnvironmentInfo("default", "parent", null); + ShouldReturn(storage, "default", expectedInfo); + + Ensemble.Stop(); + + storage.UpdateAll(); + storage.Contains("default").Should().BeTrue(); + ShouldReturn(storage, "default", expectedInfo); + } + } + [Test] public void Should_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_disabled() { @@ -187,7 +204,7 @@ public void Should_delete_environment_from_cache_if_node_was_deleted_when_observ DeleteEnvironmentNode("default"); storage.UpdateAll(); - storage.EnvironmentsInStorage.Should().Be(0); + storage.Contains("default").Should().BeFalse(); CreateEnvironmentNode("default", "parent"); storage.Get("default").Should().BeEquivalentTo(expectedInfo); diff --git a/Vostok.ServiceDiscovery/ServiceLocator.cs b/Vostok.ServiceDiscovery/ServiceLocator.cs index f58436b..228963a 100644 --- a/Vostok.ServiceDiscovery/ServiceLocator.cs +++ b/Vostok.ServiceDiscovery/ServiceLocator.cs @@ -45,7 +45,7 @@ public ServiceLocator( eventsQueue = new ActionsQueue(this.log); environmentsStorage = new EnvironmentsStorage(zooKeeperClient, pathHelper, eventsQueue, this.settings.ObserveNonExistentApplications, log); - applicationsStorage = new ApplicationsStorage(zooKeeperClient, pathHelper, eventsQueue, this.settings.ObserveNonExistentApplications, log); + applicationsStorage = new ApplicationsStorage(zooKeeperClient, pathHelper, eventsQueue, this.settings.ObserveNonExistentApplications, environmentsStorage, log); } /// diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs index 2d10db1..abad5fc 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs @@ -28,7 +28,6 @@ internal class ApplicationWithReplicas : IDisposable private readonly IZooKeeperClient zooKeeperClient; private readonly ServiceDiscoveryPathHelper pathHelper; private readonly ActionsQueue eventsQueue; - private readonly bool observeNonExistentApplication; private readonly AdHocNodeWatcher nodeWatcher; private readonly AdHocNodeWatcher existsWatcher; private readonly ILog log; @@ -50,11 +49,10 @@ public ApplicationWithReplicas( this.zooKeeperClient = zooKeeperClient; this.pathHelper = pathHelper; this.eventsQueue = eventsQueue; - this.observeNonExistentApplication = observeNonExistentApplication; this.log = log; nodeWatcher = new AdHocNodeWatcher(OnNodeEvent); - existsWatcher = this.observeNonExistentApplication ? nodeWatcher : null; + existsWatcher = observeNonExistentApplication ? nodeWatcher : null; applicationContainer = new VersionedContainer(); replicasContainer = new VersionedContainer(); } diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs index dad2be7..aaca092 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading; using Vostok.Commons.Threading; @@ -18,6 +17,7 @@ internal class ApplicationsStorage : IDisposable private readonly ServiceDiscoveryPathHelper pathHelper; private readonly ActionsQueue eventsQueue; private readonly bool observeNonExistentApplications; + private readonly EnvironmentsStorage environmentsStorage; private readonly ILog log; private readonly AtomicBoolean isDisposed = false; @@ -26,15 +26,19 @@ public ApplicationsStorage( ServiceDiscoveryPathHelper pathHelper, ActionsQueue eventsQueue, bool observeNonExistentApplications, + EnvironmentsStorage environmentsStorage, ILog log) { this.zooKeeperClient = zooKeeperClient; this.pathHelper = pathHelper; this.eventsQueue = eventsQueue; this.observeNonExistentApplications = observeNonExistentApplications; + this.environmentsStorage = environmentsStorage; this.log = log; } + public bool Contains(string environment, string application) => applications.ContainsKey((environment, application)); + public ApplicationWithReplicas Get(string environment, string application) { if (applications.TryGetValue((environment, application), out var lazy)) @@ -51,7 +55,7 @@ public void UpdateAll() return; kvp.Value.Value.Update(out var appExists); - if (!appExists && !observeNonExistentApplications) + if (!appExists && !observeNonExistentApplications && !environmentsStorage.Contains(kvp.Key.environment)) applications.TryRemove(kvp.Key, out _); } } diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs index 5ed60e9..d1d4b15 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs @@ -26,7 +26,6 @@ private readonly ConcurrentDictionary environments.Count; public EnvironmentsStorage( IZooKeeperClient zooKeeperClient, @@ -44,6 +43,8 @@ public EnvironmentsStorage( existsWatcher = this.observeNonExistentEnvironments ? nodeWatcher : null; } + public bool Contains(string environment) => environments.ContainsKey(environment); + public EnvironmentInfo Get(string name) { if (environments.TryGetValue(name, out var lazy))