Skip to content

Commit

Permalink
Don't remove app from cache if env xists
Browse files Browse the repository at this point in the history
  • Loading branch information
gladysheva committed Feb 12, 2024
1 parent 0afec84 commit 19eec5a
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 25 deletions.
99 changes: 87 additions & 12 deletions Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/ApplicationsStorage_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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++)
{
Expand All @@ -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<Uri>();

Expand All @@ -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);

Expand All @@ -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));

Expand All @@ -106,7 +105,7 @@ public void Should_store_multiple_environments_and_applications()
CreateApplicationNode("environment2", "application1", new Dictionary<string, string> {{"key", "2/1"}});
CreateApplicationNode("environment2", "application2", new Dictionary<string, string> {{"key", "2/2"}});

using (var storage = GetApplicationsStorage())
using (var storage = GetApplicationsStorage(out _))
{
ShouldReturnImmediately(
storage,
Expand Down Expand Up @@ -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<string, string>
{
Expand Down Expand Up @@ -162,7 +161,7 @@ public void Should_not_update_after_dispose()
CreateApplicationNode("default", "application", new Dictionary<string, string> {{"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<Uri> {new Uri("https://github.com/vostok")}, new Dictionary<string, string> {{"key", "value1"}});
ShouldReturnImmediately(storage, "default", "application", expected);
Expand All @@ -182,7 +181,7 @@ public void Should_not_update_to_invalid_application_properties()
CreateApplicationNode("default", "application", new Dictionary<string, string> {{"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<Uri> {new Uri("https://github.com/vostok")}, new Dictionary<string, string> {{"key", "value"}});
ShouldReturnImmediately(storage, "default", "application", expected);
Expand All @@ -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<Uri>();

Expand All @@ -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<string, string> {{"key", "1/1"}});

using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false))
{
var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary<string, string> {{"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<string, string> {{"key", "1/1"}});

using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false))
{
var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary<string, string> {{"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<string, string> {{"key", "1/1"}});

using (var storage = GetApplicationsStorage(out var envStorage, observeNonExistentApplications: false))
{
var expectedTopology = ServiceTopology.Build(new Uri[0], new Dictionary<string, string> {{"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); };
Expand All @@ -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);
}
}
}
29 changes: 23 additions & 6 deletions Vostok.ServiceDiscovery.Tests/ServiceLocatorStorage/EnvironmentsStorage_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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()
{
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Vostok.ServiceDiscovery/ServiceLocator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/// <inheritdoc />
Expand Down
4 changes: 1 addition & 3 deletions Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationWithReplicas.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<ApplicationInfo>();
replicasContainer = new VersionedContainer<Uri[]>();
}
Expand Down
8 changes: 6 additions & 2 deletions Vostok.ServiceDiscovery/ServiceLocatorStorage/ApplicationsStorage.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand All @@ -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))
Expand All @@ -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 _);
}
}
Expand Down
3 changes: 2 additions & 1 deletion Vostok.ServiceDiscovery/ServiceLocatorStorage/EnvironmentsStorage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ private readonly ConcurrentDictionary<string, Lazy<VersionedContainer<Environmen
private readonly AdHocNodeWatcher existsWatcher;
private readonly AtomicBoolean isDisposed = new AtomicBoolean(false);

public int EnvironmentsInStorage => environments.Count;

public EnvironmentsStorage(
IZooKeeperClient zooKeeperClient,
Expand All @@ -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))
Expand Down

0 comments on commit 19eec5a

Please sign in to comment.