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

Service discovery and "PollConsul" type: ServicesAreEmptyError #1634

Closed
Danimatar0 opened this issue Feb 4, 2023 · 12 comments · Fixed by #1670
Closed

Service discovery and "PollConsul" type: ServicesAreEmptyError #1634

Danimatar0 opened this issue Feb 4, 2023 · 12 comments · Fixed by #1670
Assignees
Labels
bug Identified as a potential bug merged Issue has been merged to dev and is waiting for the next release Service Discovery Ocelot feature: Service Discovery

Comments

@Danimatar0
Copy link

Danimatar0 commented Feb 4, 2023

I was trying to access a microservice called UserManagement from an ocelot api gateway but when attempting the request, i got the following error in logs:
error-consul
and here is my ocelot.json file :

{
  "Routes": [
    {
      "UseServiceDiscovery": true,
      "DownstreamPathTemplate": "/{url}",
      "DownstreamScheme": "http",
      "ServiceName": "UserManagement",
      "UpstreamPathTemplate": "/gateway/{url}",
      "UpstreamHttpMethod": [ "Get", "POST" ],
      "ReRoutesCaseSensitive": false,
      "LoadBalancerOptions": {
        "Type": "LeastConnection"
      }
    }
  ],
  "GlobalConfiguration": {
    "ServiceDiscoveryProvider": {
      "Host": "localhost",
      "Port": 8500,
      "Type": "PollConsul", // !!!
      "PollingInterval": 100
    }
  }
}

And the service was successfully registered because I can clearly see it's in consul UI dashboard in services list.
I appreciate any help because it's an urgent matter!
P.S: The following steps were produced without Docker, just a normal web API.

@ggnaegi
Copy link
Member

ggnaegi commented Apr 14, 2023

Hello please check the following issue.
#1329

Try "Consul" instead of "PollConsul"

@raman-m
Copy link
Member

raman-m commented Jun 16, 2023

@Danimatar0

I appreciate any help because it's an urgent matter !

Is it still urgent? 🤣

@raman-m
Copy link
Member

raman-m commented Jun 16, 2023

@ggnaegi Hi Guillaume!
It seems we have typo issue: Search for "PollConsul" in docs

If @Danimatar0 has used this configuration sample:

"ServiceDiscoveryProvider": {
    "Host": "localhost",
    "Port": 8500,
    "Type": "PollConsul", // WRONG !!!
    "PollingInterval": 100
}

But it is wrong!

I believe it should be:

"ServiceDiscoveryProvider": {
    "Host": "localhost",
    "Port": 8500,
    "Type": "Consul", // CORRECT !!!
    "PollingInterval": 100
}

What do you think?

@raman-m raman-m changed the title Service discovery Service discovery and "PollConsul" type: ServicesAreEmptyError Jun 16, 2023
@raman-m raman-m added bug Identified as a potential bug question Initially seen a question could become a new feature or bug or closed ;) needs validation Issue has not been replicated or verified yet needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser labels Jun 16, 2023
@ggnaegi
Copy link
Member

ggnaegi commented Jun 16, 2023

@raman-m Hi Raman
When using "Type": "Consul", then the service address is retrieved from consul for every request. When using "Type":"PollConsul", the gateway should poll Consul for service addresses, but there is imho a problem: Wenn calling the endpoint the first time, you will get the error mentioned by @Danimatar0 until the interval is reached, then you will get some answers from your endpoint... But imagine if your PollingInterval is high. Maybe the "PollConsul" variant has been abandoned. I came with another solution: https://github.com/ggnaegi/Ocelot/blob/develop/src/Ocelot.Provider.Consul/PollingConsulServiceDiscoveryProvider.cs
I could create a PR if you want, it's stable, used on a productive system for two years now.

In my case, short interval polling was too chatty, there are some issues there if your consul.json file is big with several routes. The references to the consul services are not stored as singletons. So, as soon, as you are querying new routes, then the gateway will try to retrieve the service addresses... For one address, ok, but then, it's the explosion...

@raman-m
Copy link
Member

raman-m commented Jun 16, 2023

I could create a PR if you want, it's stable, used on a productive system for two years now.

Wow! That will be awesome! 💟
You can attach this issue to your new created PR, please! Pay attention before a PR creation you need to merge all develop branch top commits to your forked develop branch as it is feature branch now! And your feature branch has a lot of merge conflicts. I've created the PR 1 to update your feature branch:

It seems it will not be easy to resolve all these merge conflicts...
The goal is to have all your changes on the top commit of ThreeMammals:develop.
The simplest strategy:

  • Backup your develop branch source code of forked repo to some safe folder
  • Remove Ocelot fork completely from your account (to remove merge conflicts implicitly 😄
  • Re-fork Ocelot with top changes in develop branch
  • Apply your code changes from the file backup

OR

I can add you to Ocelot Core team 😉 and you will have ability to create feature branches inside of Ocelot repo directly!
That will solve all stupid annoying life hacks with forked repo management!
Would you like to join Ocelot Core team? 😉


In my case, short interval polling was too chatty, there are some issues there if your consul.json file is big with several routes. The references to the consul services are not stored as singletons. So, as soon, as you are querying new routes, then the gateway will try to retrieve the service addresses... For one address, ok, but then, it's the explosion...

Oh, no! 😢
I see some critical performance issue here... 😞
Can you confirm that somehow plz? May be some sample project...
Do you have some workarounds for this bug? Have you worked on this bug?

@ggnaegi
Copy link
Member

ggnaegi commented Jun 16, 2023

Yeah, sure, I would love to be part of the team :-)

I don't have much time until wednesday, but I will prepare something next week.

As for the performance issues, they should be addressed in my forked repo.

@raman-m
Copy link
Member

raman-m commented Jun 19, 2023

Yeah, sure, I would love to be part of the team :-)

Sorry, I've double checked, and I cannot add you to the team because I have no rights to manage the repo.
I've already asked Tom. But he is silent for now.

Guillaume,
You need to decide what forked repository will you use to create future PR: your repo or my one?
I have forked Ocelot to have top commits always for linear git history.
Just let me know please!

I advise you to start by resolving merge conflicts first if you are only going to use your repo.

Waiting a good news from you about your contribution!

@ggnaegi
Copy link
Member

ggnaegi commented Jun 19, 2023

Hello @raman-m
I will delete my forked repo, refork the current official repo, import the changes to the solution, make sure the tests are passing and then I will create a PR, probably tomorrow in the afternoon or on wednesday morning.

@ggnaegi
Copy link
Member

ggnaegi commented Jun 19, 2023

By the way, the PR should address the following issues:
#1304
#1329
#1487
Some PR are related:
#1572
#1562

@raman-m
Copy link
Member

raman-m commented Jun 22, 2023

@ggnaegi

Some PR are related:
#1572
#1562

Well... Did you reuse the ideas from these PRs? 😜

@raman-m raman-m added accepted Bug or feature would be accepted as a PR or is being worked on and removed question Initially seen a question could become a new feature or bug or closed ;) needs validation Issue has not been replicated or verified yet needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser labels Jun 26, 2023
@raman-m
Copy link
Member

raman-m commented Sep 18, 2023

@Danimatar0
Could you verify the solution of linked PR #1670 please?
Let me know your testing results please!

raman-m added a commit that referenced this issue Sep 29, 2023
…ements and fix errors (#1670)

* fixing some issues in poll consul:
- Timer is not thread safe, avoiding usage of it
- No Ressources are returned for first call
- Using a providers pool, instead of creating a new provider instance

* line endings

* adding some test cases

* Using a lock instead of SemaphoreSlim

* Improve code readability

* CA2211: Non-constant fields should not be visible

* Use IOcelotLogger to remove warnings & messages of static code analysis (aka IDE0052)

* Fix errors with unit tests discovery. Remove legacy life hacks of discovering tests on .NET Core

* Update unit tests

* Also refactoring the kubernetes provider factory (like consul and eureka)

* shorten references...

* const before...

* Some minor fixes, using Equals Ordinal ignore case and a string constant for provider type definition instead of string litterals. Fixing usings.

* waiting a bit longer then?

* @RaynaldM code review

* renaming PollKubernetes to PollKube

* ... odd...

* ... very odd, we have an issue with configuration update duration...

* IDE0002: Name can be simplified

* All tests passing locally, hopefully it works online

* just a bit of cleanup

* Some missing braces and commas

* Update servicediscovery.rst: Review and update "Consul" section

---------

Co-authored-by: Guillaume Gnaegi <[email protected]>
Co-authored-by: raman-m <[email protected]>
@raman-m raman-m added merged Issue has been merged to dev and is waiting for the next release and removed accepted Bug or feature would be accepted as a PR or is being worked on labels Sep 29, 2023
@raman-m raman-m added the Service Discovery Ocelot feature: Service Discovery label Sep 29, 2023
@raman-m
Copy link
Member

raman-m commented Sep 29, 2023

@Danimatar0 @ggnaegi
Congrats! Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identified as a potential bug merged Issue has been merged to dev and is waiting for the next release Service Discovery Ocelot feature: Service Discovery
Projects
None yet
3 participants