From 7afb5a322a0066f4bdac13a7b720a2fecb9d1f82 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Fri, 2 Feb 2024 15:34:30 +0500 Subject: [PATCH 01/10] Don't set exists watches if it is not necessary --- .../ApplicationsStorage_Tests.cs | 2 +- .../EnvironmentsStorage_Tests.cs | 2 +- Vostok.ServiceDiscovery/PublicAPI.Shipped.txt | 4 +++- Vostok.ServiceDiscovery/ServiceLocator.cs | 5 ++--- .../ServiceLocatorSettings.cs | 2 ++ .../ApplicationWithReplicas.cs | 14 ++++++++++--- .../ApplicationsStorage.cs | 18 ++++++++++++---- .../EnvironmentsStorage.cs | 21 ++++++++++++++++--- 8 files changed, 52 insertions(+), 16 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index 1796312..305dbee 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -237,7 +237,7 @@ private static void ShouldReturnImmediately(ApplicationsStorage storage, string private ApplicationsStorage GetApplicationsStorage() { - return new ApplicationsStorage(ZooKeeperClient, PathHelper, EventsQueue, Log); + return new ApplicationsStorage(ZooKeeperClient, PathHelper, EventsQueue, true, 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 c80d34d..8325f82 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -168,7 +168,7 @@ private static void ShouldReturnImmediately(EnvironmentsStorage storage, string private EnvironmentsStorage GetEnvironmentsStorage() { - return new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, Log); + return new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, true, Log); } } } \ No newline at end of file diff --git a/Vostok.ServiceDiscovery/PublicAPI.Shipped.txt b/Vostok.ServiceDiscovery/PublicAPI.Shipped.txt index 0de8f3f..fb38622 100644 --- a/Vostok.ServiceDiscovery/PublicAPI.Shipped.txt +++ b/Vostok.ServiceDiscovery/PublicAPI.Shipped.txt @@ -99,4 +99,6 @@ Vostok.ServiceDiscovery.ServiceLocatorSettings.ServiceLocatorSettings() -> void Vostok.ServiceDiscovery.ServiceLocatorSettings.ZooKeeperNodesPathEscaper.get -> Vostok.ServiceDiscovery.Helpers.IZooKeeperPathEscaper Vostok.ServiceDiscovery.ServiceLocatorSettings.ZooKeeperNodesPathEscaper.set -> void Vostok.ServiceDiscovery.ServiceLocatorSettings.ZooKeeperNodesPrefix.get -> string -Vostok.ServiceDiscovery.ServiceLocatorSettings.ZooKeeperNodesPrefix.set -> void \ No newline at end of file +Vostok.ServiceDiscovery.ServiceLocatorSettings.ZooKeeperNodesPrefix.set -> void +Vostok.ServiceDiscovery.ServiceLocatorSettings.ObserveNonExistentApplications.get -> bool +Vostok.ServiceDiscovery.ServiceLocatorSettings.ObserveNonExistentApplications.set -> void \ No newline at end of file diff --git a/Vostok.ServiceDiscovery/ServiceLocator.cs b/Vostok.ServiceDiscovery/ServiceLocator.cs index daf58cb..f58436b 100644 --- a/Vostok.ServiceDiscovery/ServiceLocator.cs +++ b/Vostok.ServiceDiscovery/ServiceLocator.cs @@ -44,9 +44,8 @@ public ServiceLocator( pathHelper = new ServiceDiscoveryPathHelper(this.settings.ZooKeeperNodesPrefix, this.settings.ZooKeeperNodesPathEscaper); eventsQueue = new ActionsQueue(this.log); - environmentsStorage = new EnvironmentsStorage(zooKeeperClient, pathHelper, eventsQueue, log); - applicationsStorage = new ApplicationsStorage(zooKeeperClient, pathHelper, eventsQueue, log); - + environmentsStorage = new EnvironmentsStorage(zooKeeperClient, pathHelper, eventsQueue, this.settings.ObserveNonExistentApplications, log); + applicationsStorage = new ApplicationsStorage(zooKeeperClient, pathHelper, eventsQueue, this.settings.ObserveNonExistentApplications, log); } /// diff --git a/Vostok.ServiceDiscovery/ServiceLocatorSettings.cs b/Vostok.ServiceDiscovery/ServiceLocatorSettings.cs index b36936b..566ddd4 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorSettings.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorSettings.cs @@ -18,5 +18,7 @@ public class ServiceLocatorSettings public IZooKeeperPathEscaper ZooKeeperNodesPathEscaper { get; set; } = ZooKeeperPathEscaper.Instance; public TimeSpan IterationPeriod { get; set; } = 5.Seconds(); + + public bool ObserveNonExistentApplications { get; set; } = true; } } \ No newline at end of file diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs index 407d03b..2d10db1 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs @@ -28,7 +28,9 @@ 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; private readonly AtomicBoolean isDisposed = false; @@ -39,6 +41,7 @@ public ApplicationWithReplicas( IZooKeeperClient zooKeeperClient, ServiceDiscoveryPathHelper pathHelper, ActionsQueue eventsQueue, + bool observeNonExistentApplication, ILog log) { this.environmentName = environmentName; @@ -47,21 +50,25 @@ 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; applicationContainer = new VersionedContainer(); replicasContainer = new VersionedContainer(); } - public void Update() + public void Update(out bool appExists) { + appExists = true; + if (isDisposed) return; try { - var applicationExists = zooKeeperClient.Exists(new ExistsRequest(applicationNodePath) {Watcher = nodeWatcher}); + var applicationExists = zooKeeperClient.Exists(new ExistsRequest(applicationNodePath) {Watcher = existsWatcher}); if (!applicationExists.IsSuccessful) { return; @@ -69,6 +76,7 @@ public void Update() if (applicationExists.Stat == null) { + appExists = false; Clear(); return; } @@ -115,7 +123,7 @@ private void OnNodeEvent(NodeChangedEventType type, string path) if (isDisposed) return; - eventsQueue.Enqueue(Update); + eventsQueue.Enqueue(() => Update(out _)); } private void Clear() diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs index 633945c..dad2be7 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading; using Vostok.Commons.Threading; @@ -16,14 +17,21 @@ internal class ApplicationsStorage : IDisposable private readonly IZooKeeperClient zooKeeperClient; private readonly ServiceDiscoveryPathHelper pathHelper; private readonly ActionsQueue eventsQueue; + private readonly bool observeNonExistentApplications; private readonly ILog log; private readonly AtomicBoolean isDisposed = false; - public ApplicationsStorage(IZooKeeperClient zooKeeperClient, ServiceDiscoveryPathHelper pathHelper, ActionsQueue eventsQueue, ILog log) + public ApplicationsStorage( + IZooKeeperClient zooKeeperClient, + ServiceDiscoveryPathHelper pathHelper, + ActionsQueue eventsQueue, + bool observeNonExistentApplications, + ILog log) { this.zooKeeperClient = zooKeeperClient; this.pathHelper = pathHelper; this.eventsQueue = eventsQueue; + this.observeNonExistentApplications = observeNonExistentApplications; this.log = log; } @@ -42,7 +50,9 @@ public void UpdateAll() if (isDisposed) return; - kvp.Value.Value.Update(); + kvp.Value.Value.Update(out var appExists); + if (!appExists && !observeNonExistentApplications) + applications.TryRemove(kvp.Key, out _); } } @@ -64,9 +74,9 @@ private ApplicationWithReplicas CreateAndGet(string environment, string applicat lazy = new Lazy( () => { - var container = new ApplicationWithReplicas(environment, application, pathHelper.BuildApplicationPath(environment, application), zooKeeperClient, pathHelper, eventsQueue, log); + var container = new ApplicationWithReplicas(environment, application, pathHelper.BuildApplicationPath(environment, application), zooKeeperClient, pathHelper, eventsQueue, observeNonExistentApplications, log); if (!isDisposed) - container.Update(); + container.Update(out _); return container; }, LazyThreadSafetyMode.ExecutionAndPublication); diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs index 7d6c2fd..c68e113 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading; using Vostok.Commons.Threading; @@ -20,17 +21,26 @@ private readonly ConcurrentDictionary container) try { var environmentPath = pathHelper.BuildEnvironmentPath(name); - - var environmentExists = zooKeeperClient.Exists(new ExistsRequest(environmentPath) {Watcher = nodeWatcher}); + var environmentExists = zooKeeperClient.Exists(new ExistsRequest(environmentPath) {Watcher = existsWatcher}); if (!environmentExists.IsSuccessful) return; if (environmentExists.Stat == null) { + if (!observeNonExistentApplication) + { + environments.TryRemove(name, out _); + return; + } + container.Clear(); } else From 0afec845c173841b440330c8656321460049b6ed Mon Sep 17 00:00:00 2001 From: gladysheva Date: Mon, 12 Feb 2024 11:02:30 +0500 Subject: [PATCH 02/10] Test for env storage --- .../EnvironmentsStorage_Tests.cs | 43 ++++++++++++++++++- .../EnvironmentsStorage.cs | 13 +++--- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs index 8325f82..a049c37 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -155,6 +155,45 @@ public void UpdateAll_should_force_update() } } + [Test] + public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_enabled() + { + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: true)) + { + CreateEnvironmentNode("default", "parent"); + + var expectedInfo = new EnvironmentInfo("default", "parent", null); + storage.Get("default").Should().BeEquivalentTo(expectedInfo); + + DeleteEnvironmentNode("default"); + storage.UpdateAll(); + storage.EnvironmentsInStorage.Should().Be(1); + storage.Get("default").Should().BeNull(); + + CreateEnvironmentNode("default", "parent"); + storage.Get("default").Should().BeEquivalentTo(expectedInfo); + } + } + + [Test] + public void Should_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_disabled() + { + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: false)) + { + CreateEnvironmentNode("default", "parent"); + + var expectedInfo = new EnvironmentInfo("default", "parent", null); + storage.Get("default").Should().BeEquivalentTo(expectedInfo); + + DeleteEnvironmentNode("default"); + storage.UpdateAll(); + storage.EnvironmentsInStorage.Should().Be(0); + + CreateEnvironmentNode("default", "parent"); + storage.Get("default").Should().BeEquivalentTo(expectedInfo); + } + } + private static void ShouldReturn(EnvironmentsStorage storage, string name, EnvironmentInfo info) { Action assertion = () => { ShouldReturnImmediately(storage, name, info); }; @@ -166,9 +205,9 @@ private static void ShouldReturnImmediately(EnvironmentsStorage storage, string storage.Get(name).Should().BeEquivalentTo(info); } - private EnvironmentsStorage GetEnvironmentsStorage() + private EnvironmentsStorage GetEnvironmentsStorage(bool observeNonExistentEnvironment = true) { - return new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, true, Log); + return new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, observeNonExistentEnvironment, Log); } } } \ No newline at end of file diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs index c68e113..5ed60e9 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.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; @@ -21,26 +20,28 @@ private readonly ConcurrentDictionary environments.Count; + public EnvironmentsStorage( IZooKeeperClient zooKeeperClient, ServiceDiscoveryPathHelper pathHelper, ActionsQueue eventsHandler, - bool observeNonExistentApplication, + bool observeNonExistentEnvironments, ILog log) { this.zooKeeperClient = zooKeeperClient; this.pathHelper = pathHelper; this.eventsHandler = eventsHandler; - this.observeNonExistentApplication = observeNonExistentApplication; + this.observeNonExistentEnvironments = observeNonExistentEnvironments; this.log = log; nodeWatcher = new AdHocNodeWatcher(OnNodeEvent); - existsWatcher = this.observeNonExistentApplication ? nodeWatcher : null; + existsWatcher = this.observeNonExistentEnvironments ? nodeWatcher : null; } public EnvironmentInfo Get(string name) @@ -108,7 +109,7 @@ private void Update(string name, VersionedContainer container) if (environmentExists.Stat == null) { - if (!observeNonExistentApplication) + if (!observeNonExistentEnvironments) { environments.TryRemove(name, out _); return; From 19eec5a745d56f3f0e9803aff57e5002fb9910bf Mon Sep 17 00:00:00 2001 From: gladysheva Date: Mon, 12 Feb 2024 12:48:49 +0500 Subject: [PATCH 03/10] 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)) From 1b01a4eada9445114b07e945e693dbb9bc98dbd1 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Mon, 12 Feb 2024 13:11:51 +0500 Subject: [PATCH 04/10] Fix tests --- .../ApplicationsStorage_Tests.cs | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index 4bac018..dea2ba7 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -254,7 +254,7 @@ public void Should_not_delete_application_from_cache_when_env_exists_and_observa CreateEnvironmentNode("environment1"); CreateApplicationNode("environment1", "application1", new Dictionary {{"key", "1/1"}}); - using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) + using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: true)) { var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); ShouldReturnImmediately( @@ -276,25 +276,34 @@ public void Should_not_delete_application_from_cache_when_env_exists_and_observa [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"}}); + var environment = "environment1"; + var app = "application1"; + CreateEnvironmentNode(environment); + CreateApplicationNode(environment, app, 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", + environment, + app, expectedTopology); + envStorage.Get(environment).Should().BeEquivalentTo(new EnvironmentInfo(environment, null, null)); + Ensemble.Stop(); envStorage.UpdateAll(); - envStorage.Contains("environment1").Should().BeTrue(); + envStorage.Contains(environment).Should().BeTrue(); storage.UpdateAll(); - storage.Contains("environment1", "application1").Should().BeTrue(); + storage.Contains(environment, app).Should().BeTrue(); + ShouldReturnImmediately( + storage, + environment, + app, + expectedTopology); } } From d9891595506e12d4d95843397f2caede4242b00f Mon Sep 17 00:00:00 2001 From: gladysheva Date: Mon, 12 Feb 2024 13:25:09 +0500 Subject: [PATCH 05/10] fix test --- .../ApplicationsStorage_Tests.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index dea2ba7..625dd2f 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -251,25 +251,29 @@ public void Should_delete_application_from_cache_if_app_and_env_nodes_were_delet [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"}}); + var environment = "environment1"; + var app = "application1"; + CreateEnvironmentNode(environment); + CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: true)) { var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); ShouldReturnImmediately( storage, - "environment1", - "application1", + environment, + app, expectedTopology); - DeleteApplicationNode("environment1", "application1"); + envStorage.Get(environment).Should().BeEquivalentTo(new EnvironmentInfo(environment, null, null)); + + DeleteApplicationNode(environment, app); envStorage.UpdateAll(); - envStorage.Contains("environment1").Should().BeTrue(); + envStorage.Contains(environment).Should().BeTrue(); storage.UpdateAll(); - storage.Contains("environment1", "application1").Should().BeTrue(); + storage.Contains(environment, app).Should().BeTrue(); } } @@ -291,7 +295,7 @@ public void Should_not_delete_application_from_cache_when_observation_of_deleted expectedTopology); envStorage.Get(environment).Should().BeEquivalentTo(new EnvironmentInfo(environment, null, null)); - + Ensemble.Stop(); envStorage.UpdateAll(); From ea69c19e96ae5eff23ecdd25c2ab148b0b00bcb4 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Tue, 13 Feb 2024 12:29:19 +0500 Subject: [PATCH 06/10] Fix test --- .../ServiceLocatorStorage/EnvironmentsStorage_Tests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs index 7eb247e..139897d 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -173,11 +173,11 @@ public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_ob 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)) + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: false)) { CreateEnvironmentNode("default", "parent"); From 8df5ca23ce32638527f9c5731935af67e81a3dd0 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Tue, 13 Feb 2024 14:03:00 +0500 Subject: [PATCH 07/10] wip --- .../EnvironmentsStorage_Tests.cs | 8 ++++---- .../ServiceLocatorStorage/ApplicationWithReplicas.cs | 12 +++++++++--- .../ServiceLocatorStorage/EnvironmentsStorage.cs | 10 +++++++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs index 139897d..20fae4e 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -182,13 +182,13 @@ public void Should_not_delete_environment_from_cache_when_observation_of_deleted CreateEnvironmentNode("default", "parent"); var expectedInfo = new EnvironmentInfo("default", "parent", null); - ShouldReturn(storage, "default", expectedInfo); + ShouldReturnImmediately(storage, "default", expectedInfo); Ensemble.Stop(); storage.UpdateAll(); storage.Contains("default").Should().BeTrue(); - ShouldReturn(storage, "default", expectedInfo); + ShouldReturnImmediately(storage, "default", expectedInfo); } } @@ -200,14 +200,14 @@ public void Should_delete_environment_from_cache_if_node_was_deleted_when_observ CreateEnvironmentNode("default", "parent"); var expectedInfo = new EnvironmentInfo("default", "parent", null); - storage.Get("default").Should().BeEquivalentTo(expectedInfo); + ShouldReturnImmediately(storage, "default", expectedInfo); DeleteEnvironmentNode("default"); storage.UpdateAll(); storage.Contains("default").Should().BeFalse(); CreateEnvironmentNode("default", "parent"); - storage.Get("default").Should().BeEquivalentTo(expectedInfo); + ShouldReturnImmediately(storage, "default", expectedInfo); } } diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs index abad5fc..02d1170 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs @@ -60,7 +60,7 @@ public ApplicationWithReplicas( public void Update(out bool appExists) { appExists = true; - + if (isDisposed) return; @@ -68,9 +68,7 @@ public void Update(out bool appExists) { var applicationExists = zooKeeperClient.Exists(new ExistsRequest(applicationNodePath) {Watcher = existsWatcher}); if (!applicationExists.IsSuccessful) - { return; - } if (applicationExists.Stat == null) { @@ -83,7 +81,11 @@ public void Update(out bool appExists) { var applicationData = zooKeeperClient.GetData(new GetDataRequest(applicationNodePath) {Watcher = nodeWatcher}); if (applicationData.Status == ZooKeeperStatus.NodeNotFound) + { + appExists = false; Clear(); + } + if (!applicationData.IsSuccessful) return; @@ -96,7 +98,11 @@ public void Update(out bool appExists) { var applicationChildren = zooKeeperClient.GetChildren(new GetChildrenRequest(applicationNodePath) {Watcher = nodeWatcher}); if (applicationChildren.Status == ZooKeeperStatus.NodeNotFound) + { + appExists = false; Clear(); + } + if (!applicationChildren.IsSuccessful) return; diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs index d1d4b15..97f608d 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs @@ -26,7 +26,6 @@ private readonly ConcurrentDictionary container) var environmentData = zooKeeperClient.GetData(new GetDataRequest(environmentPath) {Watcher = nodeWatcher}); if (environmentData.Status == ZooKeeperStatus.NodeNotFound) + { + if (!observeNonExistentEnvironments) + { + environments.TryRemove(name, out _); + return; + } + container.Clear(); + } + if (!environmentData.IsSuccessful) return; From bb9dc994b43bee1042450a5ef731c56bdf184c48 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Fri, 23 Feb 2024 06:44:28 +0500 Subject: [PATCH 08/10] CR fixes --- .../ApplicationsStorage_Tests.cs | 46 +++++++++++-------- .../EnvironmentsStorage_Tests.cs | 21 +++++---- .../EnvironmentsStorage.cs | 23 ++++++---- 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index 625dd2f..8dcf07f 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -226,37 +226,47 @@ 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"}}); + const string environment = "environment1"; + const string app = "application1"; + + var expectedTopology = ServiceTopology.Build(new Uri[0], 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); + for (var i = 0; i < 10; i++) + { + CreateEnvironmentNode(environment); + CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); - DeleteApplicationNode("environment1", "application1"); - DeleteEnvironmentNode("environment1"); + envStorage.Get(environment).Should().Be(new EnvironmentInfo(environment, null, null)); - envStorage.UpdateAll(); - storage.UpdateAll(); + ShouldReturnImmediately( + storage, + environment, + app, + expectedTopology); - storage.Contains("environment1", "application1").Should().BeFalse(); + DeleteApplicationNode(environment, app); + DeleteEnvironmentNode(environment); + + envStorage.UpdateAll(); + storage.UpdateAll(); + + storage.Contains(environment, app).Should().BeFalse(); + } } } [Test] public void Should_not_delete_application_from_cache_when_env_exists_and_observation_of_deleted_apps_is_disabled() { - var environment = "environment1"; - var app = "application1"; + const string environment = "environment1"; + const string app = "application1"; + CreateEnvironmentNode(environment); CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); - using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: true)) + using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false)) { var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary {{"key", "1/1"}}); ShouldReturnImmediately( @@ -280,8 +290,8 @@ public void Should_not_delete_application_from_cache_when_env_exists_and_observa [Test] public void Should_not_delete_application_from_cache_when_observation_of_deleted_apps_is_disabled_and_client_disconnected() { - var environment = "environment1"; - var app = "application1"; + const string environment = "environment1"; + const string app = "application1"; CreateEnvironmentNode(environment); CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs index 20fae4e..62cc48c 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -173,7 +173,7 @@ public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_ob ShouldReturn(storage, "default", expectedInfo); } } - + [Test] public void Should_not_delete_environment_from_cache_when_observation_of_deleted_apps_is_disabled_and_client_disconnected() { @@ -195,19 +195,20 @@ public void Should_not_delete_environment_from_cache_when_observation_of_deleted [Test] public void Should_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_disabled() { + var expectedInfo = new EnvironmentInfo("default", "parent", null); + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: false)) { - CreateEnvironmentNode("default", "parent"); + for (var i = 0; i < 10; i++) + { + CreateEnvironmentNode("default", "parent"); - var expectedInfo = new EnvironmentInfo("default", "parent", null); - ShouldReturnImmediately(storage, "default", expectedInfo); + ShouldReturnImmediately(storage, "default", expectedInfo); - DeleteEnvironmentNode("default"); - storage.UpdateAll(); - storage.Contains("default").Should().BeFalse(); - - CreateEnvironmentNode("default", "parent"); - ShouldReturnImmediately(storage, "default", expectedInfo); + DeleteEnvironmentNode("default"); + storage.UpdateAll(); + storage.Contains("default").Should().BeFalse(); + } } } diff --git a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs index 97f608d..9853b45 100644 --- a/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs +++ b/Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs @@ -88,7 +88,9 @@ private void Update(string name) { if (!environments.TryGetValue(name, out var container)) { - log.Warn("Failed to update '{Environment}' environment: it does not exist in local cache.", name); + if (observeNonExistentEnvironments) + log.Warn("Failed to update '{Environment}' environment: it does not exist in local cache.", name); + return; } @@ -109,11 +111,8 @@ private void Update(string name, VersionedContainer container) if (environmentExists.Stat == null) { - if (!observeNonExistentEnvironments) - { - environments.TryRemove(name, out _); + if (RemoveEnvironmentFromCacheIfNeeded(name)) return; - } container.Clear(); } @@ -125,11 +124,8 @@ private void Update(string name, VersionedContainer container) var environmentData = zooKeeperClient.GetData(new GetDataRequest(environmentPath) {Watcher = nodeWatcher}); if (environmentData.Status == ZooKeeperStatus.NodeNotFound) { - if (!observeNonExistentEnvironments) - { - environments.TryRemove(name, out _); + if (RemoveEnvironmentFromCacheIfNeeded(name)) return; - } container.Clear(); } @@ -147,6 +143,15 @@ private void Update(string name, VersionedContainer container) } } + private bool RemoveEnvironmentFromCacheIfNeeded(string name) + { + if (observeNonExistentEnvironments) + return false; + + environments.TryRemove(name, out _); + return true; + } + private void OnNodeEvent(NodeChangedEventType type, string path) { if (isDisposed) From 178bd60bcded188b921ca806436a4b23d8d659d1 Mon Sep 17 00:00:00 2001 From: gladysheva Date: Fri, 23 Feb 2024 06:55:08 +0500 Subject: [PATCH 09/10] fix bug --- .../ServiceLocatorStorage/ApplicationsStorage_Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs index 8dcf07f..758c2da 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs @@ -238,7 +238,7 @@ public void Should_delete_application_from_cache_if_app_and_env_nodes_were_delet CreateEnvironmentNode(environment); CreateApplicationNode(environment, app, new Dictionary {{"key", "1/1"}}); - envStorage.Get(environment).Should().Be(new EnvironmentInfo(environment, null, null)); + envStorage.Get(environment).Should().BeEquivalentTo(new EnvironmentInfo(environment, null, null)); ShouldReturnImmediately( storage, From c11976a46247d8f20092e4c3c06e253cd4a53dbf Mon Sep 17 00:00:00 2001 From: gladysheva Date: Mon, 26 Feb 2024 10:27:55 +0500 Subject: [PATCH 10/10] Separate class for envStorages tests with observeNonExistentNodes flag --- .../EnvironmentStorage_TestsBase.cs | 26 ++++++ ...nvironmentsStorageWithObserveFlag_Tests.cs | 67 +++++++++++++++ .../EnvironmentsStorage_Tests.cs | 82 +------------------ 3 files changed, 95 insertions(+), 80 deletions(-) create mode 100644 Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentStorage_TestsBase.cs create mode 100644 Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorageWithObserveFlag_Tests.cs diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentStorage_TestsBase.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentStorage_TestsBase.cs new file mode 100644 index 0000000..677c0b7 --- /dev/null +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentStorage_TestsBase.cs @@ -0,0 +1,26 @@ +using System; +using FluentAssertions; +using Vostok.Commons.Testing; +using Vostok.ServiceDiscovery.Abstractions.Models; +using Vostok.ServiceDiscovery.ServiceLocatorStorage; + +namespace Vostok.ServiceDiscovery.Tests.ServiceLocatorStorage; + +internal class EnvironmentStorage_TestsBase : TestsBase +{ + protected static void ShouldReturn(EnvironmentsStorage storage, string name, EnvironmentInfo info) + { + Action assertion = () => { ShouldReturnImmediately(storage, name, info); }; + assertion.ShouldPassIn(DefaultTimeout); + } + + protected static void ShouldReturnImmediately(EnvironmentsStorage storage, string name, EnvironmentInfo info) + { + storage.Get(name).Should().BeEquivalentTo(info); + } + + protected EnvironmentsStorage GetEnvironmentsStorage(bool observeNonExistentEnvironment = true) + { + return new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, observeNonExistentEnvironment, Log); + } +} \ No newline at end of file diff --git a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorageWithObserveFlag_Tests.cs b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorageWithObserveFlag_Tests.cs new file mode 100644 index 0000000..35fd583 --- /dev/null +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorageWithObserveFlag_Tests.cs @@ -0,0 +1,67 @@ +using NUnit.Framework; +using FluentAssertions; +using Vostok.ServiceDiscovery.Abstractions.Models; + +namespace Vostok.ServiceDiscovery.Tests.ServiceLocatorStorage; + +[TestFixture] +internal class EnvironmentsStorageWithObserveFlag_Tests : EnvironmentStorage_TestsBase +{ + [Test] + public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_enabled() + { + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: true)) + { + CreateEnvironmentNode("default", "parent"); + + var expectedInfo = new EnvironmentInfo("default", "parent", null); + storage.Get("default").Should().BeEquivalentTo(expectedInfo); + + DeleteEnvironmentNode("default"); + storage.UpdateAll(); + storage.Contains("default").Should().BeTrue(); + storage.Get("default").Should().BeNull(); + + CreateEnvironmentNode("default", "parent"); + 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: false)) + { + CreateEnvironmentNode("default", "parent"); + + var expectedInfo = new EnvironmentInfo("default", "parent", null); + ShouldReturnImmediately(storage, "default", expectedInfo); + + Ensemble.Stop(); + + storage.UpdateAll(); + storage.Contains("default").Should().BeTrue(); + ShouldReturnImmediately(storage, "default", expectedInfo); + } + } + + [Test] + public void Should_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_disabled() + { + var expectedInfo = new EnvironmentInfo("default", "parent", null); + + using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: false)) + { + for (var i = 0; i < 10; i++) + { + CreateEnvironmentNode("default", "parent"); + + ShouldReturnImmediately(storage, "default", expectedInfo); + + DeleteEnvironmentNode("default"); + storage.UpdateAll(); + storage.Contains("default").Should().BeFalse(); + } + } + } +} \ 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 62cc48c..5f5636f 100644 --- a/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs +++ b/Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs @@ -1,16 +1,12 @@ -using System; -using System.Collections.Generic; -using FluentAssertions; +using System.Collections.Generic; using NUnit.Framework; -using Vostok.Commons.Testing; using Vostok.ServiceDiscovery.Abstractions.Models; -using Vostok.ServiceDiscovery.ServiceLocatorStorage; using Vostok.ZooKeeper.Client.Abstractions; namespace Vostok.ServiceDiscovery.Tests.ServiceLocatorStorage { [TestFixture] - internal class EnvironmentsStorage_Tests : TestsBase + internal class EnvironmentsStorage_Tests : EnvironmentStorage_TestsBase { [Test] public void Should_track_environment_parent_with_properties() @@ -153,79 +149,5 @@ public void UpdateAll_should_force_update() } } } - - [Test] - public void Should_not_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_enabled() - { - using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: true)) - { - CreateEnvironmentNode("default", "parent"); - - var expectedInfo = new EnvironmentInfo("default", "parent", null); - storage.Get("default").Should().BeEquivalentTo(expectedInfo); - - DeleteEnvironmentNode("default"); - storage.UpdateAll(); - storage.Contains("default").Should().BeTrue(); - storage.Get("default").Should().BeNull(); - - CreateEnvironmentNode("default", "parent"); - 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: false)) - { - CreateEnvironmentNode("default", "parent"); - - var expectedInfo = new EnvironmentInfo("default", "parent", null); - ShouldReturnImmediately(storage, "default", expectedInfo); - - Ensemble.Stop(); - - storage.UpdateAll(); - storage.Contains("default").Should().BeTrue(); - ShouldReturnImmediately(storage, "default", expectedInfo); - } - } - - [Test] - public void Should_delete_environment_from_cache_if_node_was_deleted_when_observation_of_deleted_apps_is_disabled() - { - var expectedInfo = new EnvironmentInfo("default", "parent", null); - - using (var storage = GetEnvironmentsStorage(observeNonExistentEnvironment: false)) - { - for (var i = 0; i < 10; i++) - { - CreateEnvironmentNode("default", "parent"); - - ShouldReturnImmediately(storage, "default", expectedInfo); - - DeleteEnvironmentNode("default"); - storage.UpdateAll(); - storage.Contains("default").Should().BeFalse(); - } - } - } - - private static void ShouldReturn(EnvironmentsStorage storage, string name, EnvironmentInfo info) - { - Action assertion = () => { ShouldReturnImmediately(storage, name, info); }; - assertion.ShouldPassIn(DefaultTimeout); - } - - private static void ShouldReturnImmediately(EnvironmentsStorage storage, string name, EnvironmentInfo info) - { - storage.Get(name).Should().BeEquivalentTo(info); - } - - private EnvironmentsStorage GetEnvironmentsStorage(bool observeNonExistentEnvironment = true) - { - return new EnvironmentsStorage(ZooKeeperClient, PathHelper, EventsQueue, observeNonExistentEnvironment, Log); - } } } \ No newline at end of file