allegro / consul-recipes Goto Github PK

URLs should be escaped in every place we construct an URL

Create typed watcher like ServiceInstancesWatcher but for watching only healthy services
https://www.consul.io/api/health.html#list-nodes-for-service

According to the Consul docs https://www.consul.io/api/features/blocking.html

Reset the index if it goes backwards. While indexes in general are monotonically increasing(i.e. they should only ever increase as time passes), there are several real-world scenarios in which they can go backwards for a given query. Implementations must check to see if a returned index is lower than the previous value, and if it is, should reset index to 0 - effectively restarting their blocking loop. Failure to do so may cause the client to miss future updates for an unbounded time, or to use an invalid index value that causes no blocking and increases load on the servers. Cases where this can occur include:

• If a raft snapshot is restored on the servers with older version of the data.
• KV list operations where an item with the highest index is removed.
• A Consul upgrade changes the way watches work to optimize them with more granular indexes.

Right now you cannot provide ACL token for leader election, which is fine when you are using Consul Agent which was configured for your purposes. It would be nice to use this recipe with general purpose agent and using specific token

ConsulWatcher.stopWatchingEndpoint(String endpoint) limits the possibility to use the watcher concurrently.

When 2 threads start watches for the same endpoint, and one decides to cancel the "subscription" by running .stopWatchingEndpoint(String Endpoint) it results in canceling both subscriptions.

Or it introduces a race condition that results in leaving both subscriptions running.

Description

Due to bug in consul. When consul agent restart, we can get wrong state - consul updates index without updating state for watched service. ( bug is already fixed in newer versions, but is present in consul 1.8.3)

There should be section that explains that consul-recipes should contain minimal number of dependencies.

With big enough initialBackoff and retry, the backoff variable can be negative.

AC:

• backoff cannot be be negative. We should set the result of initialBackoff << retry to Integer.MAX_INT if it's negative.

Although allowing stale on watches is nice because we don't have to ask a leader for a data, it can introduce many bugs.

Example.
We've got empty list of services, then we've observe new service
Events: [], [service1].
Then for a new service we run watch on its instances and if service is removed we remove the watch.

But what if we receive events in different order
Events: [service1], []
We first watch the service but we immediately unwatch it even that it's in the Consul.

Even if events are properly ordered from Consul. We can still process them in a different order. When we've got multiple threads of worker pool and synchronized operation of consuming events, the thread that got second event can run before the thread that got first event.

We should recommend for users that they either should use single threaded worker pool or save last consumed index.

What's the problem?

consul-recipes is using deprecated project embedded-consul in tests - info from embedded-consul repository:

Project has been deprecated in favour of Testcontainers Hashicorp Consul Module

What's the solution?

embedded-consul should be replaced with ConsulContainer from TestContainers.

Old CI plans were not used for almost two years and they don't work properly. We should migrate those plans to github actions and fix releasing.

The default way of creating LeaderElector described in README is:

LeaderElector elector = consulRecipes
        .leaderElector("myservice")
        .build();

This creates default consul watcher that is not closed when elector is closed. We should either close it or change the API to force user to provide external watcher.

Hi! Thank you for open sourcing this project.

I've created a Clojure wrapper around the Leader Election parts - everything is working great so far during my initial testing. One thing that I'm not sure how to change though is how long it takes to elect a new leader in a 3 node group when the leader is terminated. Currently it takes ~2 minutes for the election process to happen.
For most of our use cases that's ok, but there's a couple of scenarios where 2 minutes is way too long.

How would you go about this? Do I have to create my own instance of Session class which sets a really short session expiry (10s?)?

Thanks!