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

Max concurrent streams #4410

Closed
wants to merge 2 commits into from

Conversation

@kunalekawde
Copy link

commented Sep 24, 2019

#3806 referring this here as in 3806 git local branch was not created leading to confusion.

@bagder

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

"This branch cannot be rebased due to conflicts"

And please, squash all commits into a single one and force-push to the branch to make it easier to review!

@kunalekawde kunalekawde force-pushed the kunalekawde:max-concurrent-streams branch from 046e1fb to f0f1f97 Sep 25, 2019
Copy link
Member

left a comment

apologies for the flurry of individual comments i tried pushing the rest as a single notification

docs/libcurl/symbols-in-versions Outdated Show resolved Hide resolved
include/curl/multi.h Show resolved Hide resolved
lib/multi.c Outdated Show resolved Hide resolved
lib/multihandle.h Outdated Show resolved Hide resolved
@kunalekawde

This comment has been minimized.

Copy link
Author

commented Sep 26, 2019

apologies for the flurry of individual comments i tried pushing the rest as a single notification

Not an issue, time spent on review is more appreciated, thanks.

@jay

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

I tried to resolve one comment and somehow resolved all of them. Unwanted magic. The int_max one is resolved the others need to be addressed.

@kunalekawde

This comment has been minimized.

Copy link
Author

commented Sep 27, 2019

@jay I've updated commit with resolved comments, could you please check once, let me know if any other steps are required to take this further.
On the build failure:

  1. For Travis:
    gpg: requesting key BA9EF27F from hkp server keyserver.ubuntu.com
    gpg: keyserver timed out
    gpg: keyserver receive failed: keyserver error
  2. For Appveyor:
    a. The RPC server is unavailable
    b. Not sure if below is actually some error.
    === Start of file trace2050
    08:57:09.093000 == Info: STATE: INIT => CONNECT handle 0x1def4f093a8; line 1491 (connection #-5000)
    08:57:09.093000 == Info: Connecting to hostname: connect.example.com.2050
    08:57:09.093000 == Info: Connecting to port: 8990
    08:57:09.093000 == Info: Added connection 0. The cache now contains 1 members
    08:57:09.093000 == Info: Trying 127.0.0.1:9013...
    08:57:09.093000 == Info: TCP_NODELAY set
    08:57:09.093000 == Info: STATE: CONNECT => WAITCONNECT handle 0x1def4f093a8; line 1547 (connection #0)
    08:57:11.328000 == Info: connect to 127.0.0.1 port 9013 failed: Connection refused
    08:57:11.328000 == Info: Failed to connect to 127.0.0.1 port 9013: Connection refused
    08:57:11.328000 == Info: multi_done
    08:57:11.328000 == Info: Closing connection 0
    08:57:11.328000 == Info: The cache now contains 0 members
    === End of file trace2050
@jay

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

CI builds fail randomly due to timeouts sometimes the VM gets so slow it's inoperable. I've restarted the failed builds.

@kunalekawde

This comment has been minimized.

Copy link
Author

commented Sep 27, 2019

CI builds fail randomly due to timeouts sometimes the VM gets so slow it's inoperable. I've restarted the failed builds.

Thanks, this time the builds are successful. :)

@kunalekawde

This comment has been minimized.

Copy link
Author

commented Sep 30, 2019

@jay / @bagder , please let me know if there is any action required from my side on this.

@bagder
bagder approved these changes Sep 30, 2019
@bagder bagder requested a review from jay Sep 30, 2019
@bagder

This comment has been minimized.

Copy link
Member

commented Sep 30, 2019

@jay since you had remarks previously, I'll await your OK before I merge. I have a local version prepared in a branch now with some additional minor edits.

@bagder bagder closed this in c124e6b Oct 2, 2019
@bagder

This comment has been minimized.

Copy link
Member

commented Oct 2, 2019

Thanks @kunalekawde for your hard work on this!

@kunalekawde

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

Thanks a lot @bagder , @jay for the guidance, learnt a lot during this first PR.

@kunalekawde kunalekawde deleted the kunalekawde:max-concurrent-streams branch Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.