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

Improve recording upload backoff #42718

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Conversation

rosstimothy
Copy link
Contributor

Failed uploads could get in an infinite backoff of 10s instead of linearly backing off as intended. Due to the asynchronous nature of launching uploads by (Uploader) Scan, it could return without an error which resulted in resetting the backoff to its initial value even if previous failed uploads had incremented the backoff. To avoid this, resetting the backoff delay was modified to only occur if an upload completed successfully.

Additionally error messaging was attempted to be improved. Any errors caused by the stream being terminated should now be returned instead of a vague message.

changelog: Improve backoff of session recording uploads by teleport agents

@rosstimothy rosstimothy force-pushed the tross/recording_backoff branch from 3672a0b to daf1d9a Compare June 10, 2024 21:53
@rosstimothy rosstimothy marked this pull request as ready for review June 10, 2024 22:12
@github-actions github-actions bot requested review from AntonAM and zmb3 June 10, 2024 22:13
@github-actions github-actions bot added audit-log Issues related to Teleports Audit Log size/sm labels Jun 10, 2024
@espadolini espadolini self-requested a review June 10, 2024 22:20
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from espadolini June 11, 2024 00:14
@rosstimothy rosstimothy force-pushed the tross/recording_backoff branch from daf1d9a to 54201a7 Compare June 11, 2024 13:11
@rosstimothy rosstimothy enabled auto-merge June 11, 2024 13:12
@rosstimothy rosstimothy force-pushed the tross/recording_backoff branch 2 times, most recently from ef66eee to 2c49420 Compare June 11, 2024 14:10
Failed uploads could get in an infinite backoff of 10s instead of
linearly backing off as intended. Due to the asynchronous nature
of launching uploads by `(Uploader) Scan`, it could return without
an error which resulted in resetting the backoff to its initial
value even if previous failed uploads had incremented the backoff.
To avoid this, resetting the backoff delay was modified to only
occur if an upload completed successfully.

Additionally error messaging was attempted to be improved. Any errors
caused by the stream being terminated should now be returned instead
of a vague message.
@rosstimothy rosstimothy force-pushed the tross/recording_backoff branch from 2c49420 to 83b9045 Compare June 11, 2024 14:38
@rosstimothy rosstimothy added this pull request to the merge queue Jun 11, 2024
Merged via the queue into master with commit e33de20 Jun 11, 2024
37 checks passed
@rosstimothy rosstimothy deleted the tross/recording_backoff branch June 11, 2024 15:35
@public-teleport-github-review-bot

@rosstimothy See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed
branch/v16 Create PR

rosstimothy added a commit that referenced this pull request Jun 11, 2024
Failed uploads could get in an infinite backoff of 10s instead of
linearly backing off as intended. Due to the asynchronous nature
of launching uploads by `(Uploader) Scan`, it could return without
an error which resulted in resetting the backoff to its initial
value even if previous failed uploads had incremented the backoff.
To avoid this, resetting the backoff delay was modified to only
occur if an upload completed successfully.

Additionally error messaging was attempted to be improved. Any errors
caused by the stream being terminated should now be returned instead
of a vague message.
rosstimothy added a commit that referenced this pull request Jun 11, 2024
Failed uploads could get in an infinite backoff of 10s instead of
linearly backing off as intended. Due to the asynchronous nature
of launching uploads by `(Uploader) Scan`, it could return without
an error which resulted in resetting the backoff to its initial
value even if previous failed uploads had incremented the backoff.
To avoid this, resetting the backoff delay was modified to only
occur if an upload completed successfully.

Additionally error messaging was attempted to be improved. Any errors
caused by the stream being terminated should now be returned instead
of a vague message.
github-merge-queue bot pushed a commit that referenced this pull request Jun 11, 2024
Failed uploads could get in an infinite backoff of 10s instead of
linearly backing off as intended. Due to the asynchronous nature
of launching uploads by `(Uploader) Scan`, it could return without
an error which resulted in resetting the backoff to its initial
value even if previous failed uploads had incremented the backoff.
To avoid this, resetting the backoff delay was modified to only
occur if an upload completed successfully.

Additionally error messaging was attempted to be improved. Any errors
caused by the stream being terminated should now be returned instead
of a vague message.
github-merge-queue bot pushed a commit that referenced this pull request Jun 11, 2024
Failed uploads could get in an infinite backoff of 10s instead of
linearly backing off as intended. Due to the asynchronous nature
of launching uploads by `(Uploader) Scan`, it could return without
an error which resulted in resetting the backoff to its initial
value even if previous failed uploads had incremented the backoff.
To avoid this, resetting the backoff delay was modified to only
occur if an upload completed successfully.

Additionally error messaging was attempted to be improved. Any errors
caused by the stream being terminated should now be returned instead
of a vague message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants