Skip to content

Conversation

@Whaileee
Copy link
Contributor

@Whaileee Whaileee commented Apr 9, 2025

No description provided.

@Whaileee Whaileee requested review from IngelaAndin and u3s April 9, 2025 18:09
@Whaileee Whaileee added testing currently being tested, tag is used by OTP internal CI team:PS Assigned to OTP team PS labels Apr 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2025

CT Test Results

  2 files   23 suites   10m 58s ⏱️
367 tests 360 ✅  7 💤 0 ❌
749 runs  664 ✅ 85 💤 0 ❌

Results for commit ec7eab2.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 2 times, most recently from 2a8a4ad to 4e3f24f Compare April 10, 2025 12:02
@Whaileee Whaileee self-assigned this Apr 10, 2025
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch from 4e3f24f to 92f73a1 Compare April 10, 2025 15:17
@Whaileee
Copy link
Contributor Author

@github-actions disable-cache

@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 2 times, most recently from 465413d to 5e33cc0 Compare April 10, 2025 16:11
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 2 times, most recently from 7acd444 to 2e53f7f Compare April 11, 2025 12:21
@Whaileee
Copy link
Contributor Author

solves #8841

@Whaileee Whaileee added the full-build-and-check Run a more thorough testing of this PR label Apr 11, 2025
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch from 2e53f7f to f1603f9 Compare April 11, 2025 15:07
@Whaileee Whaileee removed the full-build-and-check Run a more thorough testing of this PR label May 29, 2025
@Whaileee Whaileee marked this pull request as ready for review July 17, 2025 09:05
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch from bd74dab to 2ca8d94 Compare July 17, 2025 09:18
@Whaileee Whaileee changed the base branch from master to maint July 17, 2025 09:44
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 2 times, most recently from 6b02113 to 2974eac Compare July 17, 2025 09:54
@Whaileee Whaileee added the full-build-and-check Run a more thorough testing of this PR label Jul 21, 2025
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 3 times, most recently from f143915 to d8a005b Compare July 21, 2025 09:44
@Whaileee Whaileee requested a review from Copilot July 21, 2025 09:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new option max_connections_open for the HTTP client profile to limit the maximum number of concurrent HTTP handlers that can be open simultaneously. This feature helps prevent bandwidth exhaustion when multiple requests are processed concurrently and remote servers close connections before requests complete.

  • Adds max_connections_open option to limit concurrent HTTP handlers
  • Implements request queuing mechanism when handler limit is reached
  • Adds comprehensive test coverage for high-load scenarios with both HTTP and HTTPS

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
system/doc/general_info/upcoming_incompatibilities.md Documents the new option as an upcoming feature
lib/inets/src/http_client/httpc_internal.hrl Adds max_connections_open field to options record
lib/inets/src/http_client/httpc.erl Implements option validation and documentation
lib/inets/src/http_client/httpc_manager.erl Core logic for handler limiting and request queuing
lib/inets/test/httpc_SUITE.erl Comprehensive test suite for high-load scenarios
lib/inets/test/http_test_lib.erl Minor test infrastructure update
lib/inets/src/http_client/httpc_response.erl Cache renewal marker
lib/inets/src/http_client/httpc_request.erl Cache renewal marker
lib/inets/src/http_client/httpc_handler.erl Cache renewal marker
lib/inets/src/http_client/httpc_cookie.erl Cache renewal marker
Comments suppressed due to low confidence (2)

system/doc/general_info/upcoming_incompatibilities.md:259

  • The option name in documentation should match the actual implementation. Use max_connections_open consistently throughout.
New option in Inets' http client `httpc:set_options([{max_connections_open, N}])`

lib/inets/src/http_client/httpc.erl:1649

  • [nitpick] Function name is inconsistent with the option name max_connections_open. Consider renaming to validate_max_sessions_max_handlers_open or updating the option name for consistency.
validate_max_sessions_max_connections_open(MaxSessions, MaxConnectionsOpen)

@Whaileee Whaileee added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Aug 20, 2025
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch from 2eebe76 to 40ab8e6 Compare August 25, 2025 11:35
@Whaileee Whaileee added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Aug 25, 2025
@IngelaAndin IngelaAndin changed the title add max_handlers_open option for httpc profile add max_connections_open option for httpc profile Aug 25, 2025
IngelaAndin
IngelaAndin previously approved these changes Aug 25, 2025
u3s
u3s previously approved these changes Aug 25, 2025
validate_max_sessions(Value),
validate_options(Tail, [Opt | Acc]);

validate_options([{max_connections_open, Value} = Opt| Tail], Acc) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

space missing before |

-doc(#{equiv => set_options/2}).
-doc(#{since => <<"OTP R13B04">>}).
-spec set_options(Options) -> ok | {error, Reason} when
Options :: [Option],
Copy link
Contributor

Choose a reason for hiding this comment

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

general comment: please squash commits before merging (unless there is some information in how they're split now)

Comment on lines 1748 to 1749
(is_integer(Value) andalso (Value > 0)) orelse
Value =:= infinity ->
Copy link
Contributor

Choose a reason for hiding this comment

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

  • combine 2 lines above into 1 - so it is same as in function above and below?
  • if you want to multiline guard - format it same as in other functions in this module - e.g. validate_proxy
  • reformat code, indentation is wrong. guards should have more indentation, as on screenshot below
Image

@Whaileee Whaileee dismissed stale reviews from u3s and IngelaAndin via b4e9bef August 28, 2025 11:40
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 6 times, most recently from 2e3f864 to b9bf05f Compare September 4, 2025 08:29
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 4 times, most recently from 8e04927 to d556ef4 Compare September 9, 2025 14:03
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch 4 times, most recently from 7a10c76 to 9afb929 Compare October 6, 2025 11:04
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch from 9afb929 to 8c3bc1e Compare December 18, 2025 12:22
@Whaileee Whaileee force-pushed the whaileee/inets/httpc/socket_closed_remotely/GH-8841/OTP-19587 branch from 8c3bc1e to ec7eab2 Compare December 18, 2025 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants