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

Default timeout of HttpClient via route configuration #1869

Open
RightDecision2 opened this issue Dec 26, 2023 · 14 comments
Open

Default timeout of HttpClient via route configuration #1869

RightDecision2 opened this issue Dec 26, 2023 · 14 comments
Labels
Configuration Ocelot feature: Configuration low Low priority proposal Proposal for a new functionality in Ocelot QoS Ocelot feature: Quality of Service

Comments

@RightDecision2
Copy link

Expected Behavior / New Feature

// HttpClientBuilder.cs
_defaultTimeout = TimeSpan.FromSeconds(_configuration.TimeOut);

Actual Behavior / Motivation for New Feature

// HttpClientBuilder.cs
_defaultTimeout = TimeSpan.FromSeconds(90);

Specifications

  • Version: All
  • Platform: All
  • Subsystem: Any
@raman-m
Copy link
Member

raman-m commented Dec 26, 2023

Are you going to create a PR these days?

@RightDecision2
Copy link
Author

RightDecision2 commented Dec 26, 2023 via email

@raman-m
Copy link
Member

raman-m commented Dec 26, 2023

What is the problem with default 90 seconds timeout?

As a quick fix use QoS TimeoutValue property to have custom timeout value.
I don't think that performance will be decreased dramatically: a few microseconds will be added 😄

@raman-m
Copy link
Member

raman-m commented Dec 26, 2023

@RightDecision2 commented on Dec 26

👌 Great! Waiting for your PR creation... Will review with pleasure!
But you need to fork Ocelot repository first!
Your repos are empty now.

@raman-m raman-m added QoS Ocelot feature: Quality of Service Configuration Ocelot feature: Configuration proposal Proposal for a new functionality in Ocelot labels Dec 26, 2023
@raman-m raman-m changed the title Add to configuration hardcoded timeout of HttpClient Add HttpClient timeout to route configuration Dec 26, 2023
@raman-m raman-m changed the title Add HttpClient timeout to route configuration Default timeout of HttpClient via route configuration Dec 26, 2023
@RightDecision2
Copy link
Author

RightDecision2 commented Dec 27, 2023 via email

@raman-m
Copy link
Member

raman-m commented Dec 27, 2023

Dear author, What's your name?
What's your LinkedIn?

@raman-m
Copy link
Member

raman-m commented Dec 27, 2023

I have OCR service, which process pdf file. Every request average 2mins.
When I use QoS its random cancelled after 90 seconds (few times might be worked).

Strange cancellation of your requests by QoS logic... You need to show this behavior by sharing to us Ocelot logs.
It should not be cancelled randomly... All requests should be done without problems if TimeoutValue in QoS is greater than 2 mins (average processing time by your downstream service). If some requests failed, that means root cause is not on Ocelot's side...

What is the maximum processing time? Seems you have to define max value in route config.
What exact value of TimeoutValue?

Please attach route JSON!

@raman-m
Copy link
Member

raman-m commented Dec 27, 2023

So for me, is only one possible solution - increase default timeout. I think for someone it could be useful to.

I'm not sure it will help you...
How stable your downstream service if you connect directly (without Ocelot routing)?

As we've agreed on, you are going to create a PR soon to manage default timeout from route config, right?
Could you Sync fork please?
Our development process requires to fork the Ocelot repo first.

@raman-m raman-m added the low Low priority label Jan 7, 2024
@RightDecision2
Copy link
Author

RightDecision2 commented Jan 9, 2024 via email

@lemosluan
Copy link

lemosluan commented Feb 8, 2024

The doc from ocelot's site says that there is a timeout to 90s.
You can use Polly to reach a new timeout, but in some cases, the response from our services take more than 90s, so it would be useful if the timeout on startup of ocelot can be set.

image

@raman-m
Copy link
Member

raman-m commented Feb 22, 2024

@RightDecision2 commented on Jan 9

How is your development going?
I guess you should be interested in fixing your problem...
But sorry, as a team, we are busy with other more important tasks. So, you need to fix by your own and create a PR, if you wish to contribute.

Will you contribute or not?

Можешь писать на русском языке, если это ускорит нашу разработку.
Не люблю, когда скрываются под всякими анонимными никами, да ещё и отвечают с мобилы, где установлен русскоязычный почтовый клиент.
И я задавал этот вопрос
Зачем шифроваться под левой учёткой ГитХаба? Не понимаю этого! Наверняка есть основной аккаунт, другой...

@raman-m
Copy link
Member

raman-m commented Feb 22, 2024

@lemosluan commented on Feb 8

Hi Luan!
What's your solution for Ocelot pipeline setup?

This author's problem can be fixed by a Delegating Handler.
A few lines of code in the body of the handler. Something like that:

public class TimeoutHandler : DelegatingHandler
{
    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken token)
    {
            TimeSpan? timeout = TimeSpan.FromHours(1.0); // hardcoded but can be read from the config
            request.Properties["RequestTimeout"] = timeout;
            return await base.SendAsync(request, token);
    }
}

But I'm not sure on design correctness, it should be checked.
Personally I like the following robust solution: Better timeout handling with HttpClient

And after that, just attach TimeoutHandler to Ocelot pipeline as:

ConfigureServices(s => s
    .AddOcelot()
    .AddDelegatingHandler<TimeoutHandler>()
)

and route config is

{
  // other props
  "DelegatingHandlers": ["TimeoutHandler"]
}

@RightDecision2
Copy link
Author

RightDecision2 commented Feb 23, 2024 via email

@raman-m
Copy link
Member

raman-m commented Feb 23, 2024

@RightDecision2 Приоритет низкий!
Без понятия в какой релиз это попадёт. Если кто-то создаст ПР, то фича появится быстрее.
Ждите! Может кто-то контрибьютнет...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Configuration Ocelot feature: Configuration low Low priority proposal Proposal for a new functionality in Ocelot QoS Ocelot feature: Quality of Service
Projects
None yet
Development

No branches or pull requests

3 participants