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

1656 upload timeouts #2122

Merged
merged 8 commits into from
Dec 6, 2021
Merged

1656 upload timeouts #2122

merged 8 commits into from
Dec 6, 2021

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Dec 1, 2021

What does this pull request do? Explain your changes. (required)
While making an HTTP request from B -> O, introduce a separate timeout for the segment data upload. Before there was one timeout for the whole request.

Specific updates (required)

  • Add timeout for segment upload + unit tests

How did you test each of these updates (required)
Check locally if transcoding (still) works. Then, introduced an artificial sleep in O and tried again. Note the timeout error message in B.

Does this pull request close any open issues?
fix #1656

Checklist:

Copy link
Contributor

@darkdarkdragon darkdarkdragon left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@victorges victorges left a comment

Choose a reason for hiding this comment

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

LGTM overall, but I think the core sendReqWithTimeout has some issues that could be improved.

server/segment_rpc.go Outdated Show resolved Hide resolved
server/segment_rpc.go Show resolved Hide resolved
server/segment_rpc.go Outdated Show resolved Hide resolved
server/segment_rpc_test.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Thanks for the review @victorges . Added my comments.

server/segment_rpc.go Outdated Show resolved Hide resolved
server/segment_rpc.go Show resolved Hide resolved
server/segment_rpc.go Outdated Show resolved Hide resolved
server/segment_rpc_test.go Outdated Show resolved Hide resolved
@leszko leszko requested a review from victorges December 3, 2021 11:22
Copy link
Member

@victorges victorges left a comment

Choose a reason for hiding this comment

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

LGTM!

server/segment_rpc.go Show resolved Hide resolved
@leszko leszko merged commit 6a72cc3 into livepeer:master Dec 6, 2021
@leszko leszko deleted the 1656-upload-timeouts branch December 6, 2021 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

B <> O upload timeouts
3 participants