Skip to content

feat: adapt health check timeout algorithm#1559

Merged
RobPasMue merged 6 commits intomainfrom
feat/health-check-timeout
Nov 28, 2024
Merged

feat: adapt health check timeout algorithm#1559
RobPasMue merged 6 commits intomainfrom
feat/health-check-timeout

Conversation

@RobPasMue
Copy link
Member

Description

As title says - timeout for health check is adapted more intelligently by backing off to double the previous timeout... until we reach the timeout limit set by the user. This allows us to adapt to "slow connections" and also avoid exposing parameters. These parameters would be hard to control by users and not expected really.

Issue linked

Closes #1558

At least the main problem in #1558 - we won't be exposing the parameter in the end.

Checklist

  • I have tested my changes locally.
  • I have added necessary documentation or updated existing documentation.
  • I have followed the coding style guidelines of this project.
  • I have added appropriate unit tests.
  • I have reviewed my changes before submitting this pull request.
  • I have linked the issue or issues that are solved to the PR if any.
  • I have assigned this PR to myself.
  • I have added the minimum version decorator to any new backend method implemented.
  • I have made sure that the title of my PR follows Conventional commits style (e.g. feat: extrude circle to cylinder)

@RobPasMue RobPasMue requested a review from a team as a code owner November 28, 2024 12:05
@RobPasMue RobPasMue self-assigned this Nov 28, 2024
@github-actions github-actions bot added the enhancement New features or code improvements label Nov 28, 2024
@codecov
Copy link

codecov bot commented Nov 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.60%. Comparing base (7ab4a01) to head (4ef8e5c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1559   +/-   ##
=======================================
  Coverage   91.60%   91.60%           
=======================================
  Files          86       86           
  Lines        7051     7055    +4     
=======================================
+ Hits         6459     6463    +4     
  Misses        592      592           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@AlejandroFernandezLuces AlejandroFernandezLuces left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just FYI, we have an exponential backoff algorithm implemented in the LLM handler. I think for this case this is enough, but in case having less connection tries is required at some point we can consider applying it here.

@RobPasMue
Copy link
Member Author

LGTM! Just FYI, we have an exponential backoff algorithm implemented in the LLM handler. I think for this case this is enough, but in case having less connection tries is required at some point we can consider applying it here.

Noted, thanks! I don't think the exponential part is needed here. But worth keeping in mind.

@RobPasMue RobPasMue merged commit 5b1647c into main Nov 28, 2024
@RobPasMue RobPasMue deleted the feat/health-check-timeout branch November 28, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New features or code improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Health check timeout is too short for remote connections

3 participants