Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Fix exponent overflow #100

Merged
merged 8 commits into from
Mar 31, 2020
Merged

Fix exponent overflow #100

merged 8 commits into from
Mar 31, 2020

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Mar 31, 2020

TL;DR

Backoff controller has two issues: 1) backoff() can overflow the exponent value leading to 0 backoff duration all the time. 2) There is a race condition in updating fields of the simple backoff handler. this PR should address both.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

flyteorg/flyte#233

@bnsblue
Copy link
Contributor

bnsblue commented Mar 31, 2020

Not sure if a lock is really needed in backoff handler because the race condition should be benign (increment and reset to 0 only). But if we don't see much performance hit we can keep it just to be safe.

bnsblue
bnsblue previously approved these changes Mar 31, 2020
Copy link
Contributor

@bnsblue bnsblue left a comment

Choose a reason for hiding this comment

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

should we add some debug msg in backoff to print out the exponent, base, and max?

@codecov-io
Copy link

codecov-io commented Mar 31, 2020

Codecov Report

Merging #100 into master will increase coverage by 0.11%.
The diff coverage is 64.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   49.82%   49.94%   +0.11%     
==========================================
  Files         127      127              
  Lines        7970     7959      -11     
==========================================
+ Hits         3971     3975       +4     
+ Misses       3611     3596      -15     
  Partials      388      388              
Impacted Files Coverage Δ
pkg/controller/nodes/task/backoff/controller.go 0.00% <0.00%> (ø)
pkg/controller/nodes/task/backoff/handler.go 73.91% <84.61%> (+1.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1e9ca2...cbaddb9. Read the comment docs.

kumare3
kumare3 previously approved these changes Mar 31, 2020
@EngHabu EngHabu merged commit fd3571d into master Mar 31, 2020
eapolinario pushed a commit to eapolinario/flytepropeller that referenced this pull request Aug 9, 2023
Backoff controller has two issues: 1) backoff() can overflow the exponent value leading to 0 backoff duration all the time. 2) There is a race condition in updating fields of the simple backoff handler. this PR should address both.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants