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

libgit2: enforce context timeout #740

Merged
merged 1 commit into from May 27, 2022
Merged

libgit2: enforce context timeout #740

merged 1 commit into from May 27, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented May 27, 2022

Some scenarios could lead a goroutine to be running indefinetely within managed ssh.
Previously between the two git operations, the reconciliation could take twice the timeout set for the Flux object.

After the changes were introduced (after 10pm), the reconciliations became more stable:
image

@pjbgf pjbgf added the area/git Git related issues and pull requests label May 27, 2022
@pjbgf pjbgf added this to the GA milestone May 27, 2022
@pjbgf pjbgf requested a review from aryan9600 May 27, 2022 09:41
@pjbgf pjbgf force-pushed the ssh-context branch 4 times, most recently from ed16205 to bc011ee Compare May 27, 2022 12:28

// When context is nil, creates a new with internal SSH connection timeout.
if ctx == nil {
ctx, cancel = context.WithTimeout(context.TODO(), sshConnectionTimeOut)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ctx, cancel = context.WithTimeout(context.TODO(), sshConnectionTimeOut)
ctx, cancel = context.WithTimeout(context.Background(), sshConnectionTimeOut)

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @pjbgf

Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

LGTM!

Some scenarios could lead a goroutine to be running indefinetely within managed ssh.
Previously between the two git operations, the reconciliation
could take twice the timeout set for the Flux object.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants