multi: fix timing metrics on connect failure #14204

jmaggard10 · 2024-07-16T22:53:52Z

On platforms without AsynchDNS, if name resolution fails for a transaction, CURLINFO_TOTAL_TIME (and all other timing metrics) return 0, which is rather misleading.

This commit adds a final Curl_pgrsUpdate() call before returning to update the timing metrics.

dfandrich · 2024-07-18T18:02:13Z

Analysis of PR #14204:

Test 1268 failed,
which has NOT been flaky recently, so there could be a real issue in the PR.
Note that this test has failed in 70 different CI jobs (the link just goes to one of them).

Test 1471 failed,
which has NOT been flaky recently, so there could be a real issue in the PR.
Note that this test has failed in 70 different CI jobs (the link just goes to one of them).

Test 1472 failed,
which has NOT been flaky recently, so there could be a real issue in the PR.
Note that this test has failed in 70 different CI jobs (the link just goes to one of them).

Generated by Testclutch

dfandrich · 2024-07-18T18:04:28Z

Several tests aren't expecting to see progress output in their stderr, so those should be updated to add --no-progress-meter or similar.

bagder

Test fails need attention

On platforms without AsynchDNS, if name resolution fails for a transaction, CURLINFO_TOTAL_TIME (and all other timing metrics) return 0, which is rather misleading. This commit adds a final Curl_pgrsUpdate() call before returning to update the timing metrics.

jmaggard10 · 2024-07-19T22:30:24Z

Tests fixed and passing now.

bagder · 2024-07-19T22:35:15Z

Hm, I'm having second thoughts about this. It might upset a few people as it now (uslessly) outputs that progress bar in failure cases is never did before...

To fix timing metric etc Co-authored-by: Justin Maggard Fixes #14204

bagder · 2024-07-19T22:49:19Z

I propose #14239 instead. It should get the same effect but without the progress callback/meter update.

jmaggard10 · 2024-07-19T22:51:08Z

Sounds good to me, thanks.

To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239

* multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * conncache: connection shutdown, multi_socket handling - implement the socket hash user/reader/writer processing also for connections that are being shut down by the connection cache. - split out handling of current vs. last pollset socket event handling into a function available in other code parts - add `shutdown_poll` pollset to `connectdata` struct so that changes in the pollset can be recorded during shutdown. (The internal handle cannot keep it since it might be used for many connections) Reported-by: calvin2021y on github Fixes curl#14252 Closes curl#14257 * GHA: bump github/codeql-action from 3.25.11 to 3.25.13 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.11 to 3.25.13. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@b611370...2d79040) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Closes curl#14255 * docs/libcurl: add to cleanup docs that their inputs go invalid Reported-by: icy17 on github Fixes curl#14248 Closes curl#14258 * curl_easy_escape.md: move historic details to HISTORY Closes curl#14261 * THANKS: updates from the 8.9.0 release * RELEASE-NOTES: synced * RELEASE-PROCEDURE.md: remove the initial build step Because it is no longer needed to be done by a person as the dmaketgz script does it by itself. Removed two past release dates, added two new future ones Closes curl#14267 * RELEASE-NOTES: synced bumped to 8.9.1 * RELEASE-PROCEDURE.md: restore next release date Pointed-out-by: extrimexxx on github Bug: curl#14267 (comment) --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * develop (#316) * multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * conncache: connection shutdown, multi_socket handling - implement the socket hash user/reader/writer processing also for connections that are being shut down by the connection cache. - split out handling of current vs. last pollset socket event handling into a function available in other code parts - add `shutdown_poll` pollset to `connectdata` struct so that changes in the pollset can be recorded during shutdown. (The internal handle cannot keep it since it might be used for many connections) Reported-by: calvin2021y on github Fixes curl#14252 Closes curl#14257 * GHA: bump github/codeql-action from 3.25.11 to 3.25.13 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.11 to 3.25.13. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@b611370...2d79040) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Closes curl#14255 * docs/libcurl: add to cleanup docs that their inputs go invalid Reported-by: icy17 on github Fixes curl#14248 Closes curl#14258 * curl_easy_escape.md: move historic details to HISTORY Closes curl#14261 * THANKS: updates from the 8.9.0 release * RELEASE-NOTES: synced * RELEASE-PROCEDURE.md: remove the initial build step Because it is no longer needed to be done by a person as the dmaketgz script does it by itself. Removed two past release dates, added two new future ones Closes curl#14267 * RELEASE-NOTES: synced bumped to 8.9.1 * RELEASE-PROCEDURE.md: restore next release date Pointed-out-by: extrimexxx on github Bug: curl#14267 (comment) --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Create cmake-multi-platform.yml * multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * develop (#317) * multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * conncache: connection shutdown, multi_socket handling - implement the socket hash user/reader/writer processing also for connections that are being shut down by the connection cache. - split out handling of current vs. last pollset socket event handling into a function available in other code parts - add `shutdown_poll` pollset to `connectdata` struct so that changes in the pollset can be recorded during shutdown. (The internal handle cannot keep it since it might be used for many connections) Reported-by: calvin2021y on github Fixes curl#14252 Closes curl#14257 * GHA: bump github/codeql-action from 3.25.11 to 3.25.13 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.11 to 3.25.13. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@b611370...2d79040) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Closes curl#14255 * docs/libcurl: add to cleanup docs that their inputs go invalid Reported-by: icy17 on github Fixes curl#14248 Closes curl#14258 * curl_easy_escape.md: move historic details to HISTORY Closes curl#14261 * THANKS: updates from the 8.9.0 release * RELEASE-NOTES: synced * RELEASE-PROCEDURE.md: remove the initial build step Because it is no longer needed to be done by a person as the dmaketgz script does it by itself. Removed two past release dates, added two new future ones Closes curl#14267 * RELEASE-NOTES: synced bumped to 8.9.1 * RELEASE-PROCEDURE.md: restore next release date Pointed-out-by: extrimexxx on github Bug: curl#14267 (comment) --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Co-authored-by: بلال مسلوب <145295387+mesloubi2021@users.noreply.github.com> Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Create c-cpp.yml * multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * develop (#318) * multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * conncache: connection shutdown, multi_socket handling - implement the socket hash user/reader/writer processing also for connections that are being shut down by the connection cache. - split out handling of current vs. last pollset socket event handling into a function available in other code parts - add `shutdown_poll` pollset to `connectdata` struct so that changes in the pollset can be recorded during shutdown. (The internal handle cannot keep it since it might be used for many connections) Reported-by: calvin2021y on github Fixes curl#14252 Closes curl#14257 * GHA: bump github/codeql-action from 3.25.11 to 3.25.13 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.11 to 3.25.13. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@b611370...2d79040) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Closes curl#14255 * docs/libcurl: add to cleanup docs that their inputs go invalid Reported-by: icy17 on github Fixes curl#14248 Closes curl#14258 * curl_easy_escape.md: move historic details to HISTORY Closes curl#14261 * THANKS: updates from the 8.9.0 release * RELEASE-NOTES: synced * RELEASE-PROCEDURE.md: remove the initial build step Because it is no longer needed to be done by a person as the dmaketgz script does it by itself. Removed two past release dates, added two new future ones Closes curl#14267 * RELEASE-NOTES: synced bumped to 8.9.1 * RELEASE-PROCEDURE.md: restore next release date Pointed-out-by: extrimexxx on github Bug: curl#14267 (comment) --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Co-authored-by: بلال مسلوب <145295387+mesloubi2021@users.noreply.github.com> Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * develop (#319) * multi: do a final progress update on connect failure To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239 * CURLOPT_SSL_VERIFYHOST.md: refresh Move mentions of old behavior to the HISTORY section to make it easier to read about modern behavior. Added a MATCHING section. Closes curl#14241 * lib: Curl_posttransfer => multi_posttransfer Moved from transfer.c to multi.c as it was only used within multi.c Made a void, as it returned a fixed return code nothing checked. Closes curl#14240 * tool_cb_prg: output "flying saucers" with leading carriage return Because that is how the progress-bar is output, so when the progress-bar has been shown at least once and the information is reset, like for a redirect, there might be a moment where the size goes from known to unknown and then the flying saucerts are shown after a brief display of the progress-bar. It could previously cause accidental character leftovers on the right side of the bar when using a narrow display. Reported-by: Chris Webb Fixes curl#14213 Closes curl#14246 * conncache: connection shutdown, multi_socket handling - implement the socket hash user/reader/writer processing also for connections that are being shut down by the connection cache. - split out handling of current vs. last pollset socket event handling into a function available in other code parts - add `shutdown_poll` pollset to `connectdata` struct so that changes in the pollset can be recorded during shutdown. (The internal handle cannot keep it since it might be used for many connections) Reported-by: calvin2021y on github Fixes curl#14252 Closes curl#14257 * GHA: bump github/codeql-action from 3.25.11 to 3.25.13 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.11 to 3.25.13. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@b611370...2d79040) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Closes curl#14255 * docs/libcurl: add to cleanup docs that their inputs go invalid Reported-by: icy17 on github Fixes curl#14248 Closes curl#14258 * curl_easy_escape.md: move historic details to HISTORY Closes curl#14261 * THANKS: updates from the 8.9.0 release * RELEASE-NOTES: synced * RELEASE-PROCEDURE.md: remove the initial build step Because it is no longer needed to be done by a person as the dmaketgz script does it by itself. Removed two past release dates, added two new future ones Closes curl#14267 * RELEASE-NOTES: synced bumped to 8.9.1 * RELEASE-PROCEDURE.md: restore next release date Pointed-out-by: extrimexxx on github Bug: curl#14267 (comment) --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Co-authored-by: Daniel Stenberg <daniel@haxx.se> Co-authored-by: Stefan Eissing <stefan@eissing.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

bagder approved these changes Jul 18, 2024

View reviewed changes

bagder requested changes Jul 18, 2024

View reviewed changes

jmaggard10 force-pushed the fix-timing-metrics-on-resolve-failure branch from b95f918 to c4583f6 Compare July 18, 2024 23:15

jmaggard10 force-pushed the fix-timing-metrics-on-resolve-failure branch from c4583f6 to df9aaf3 Compare July 18, 2024 23:16

github-actions bot added the tests label Jul 18, 2024

jmaggard10 requested a review from bagder July 19, 2024 22:30

bagder approved these changes Jul 19, 2024

View reviewed changes

bagder added a commit that referenced this pull request Jul 19, 2024

multi: do a final progress update on connect failure

f54fc91

To fix timing metric etc Co-authored-by: Justin Maggard Fixes #14204

bagder mentioned this pull request Jul 19, 2024

multi: do a final progress update on connect failure #14239

Closed

bagder closed this in 56493ee Jul 20, 2024

meslubi2021 pushed a commit to Unity-Curl/curl that referenced this pull request Jul 22, 2024

multi: do a final progress update on connect failure

0197fae

To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239

meslubi2021 pushed a commit to Unity-Curl/curl that referenced this pull request Jul 22, 2024

multi: do a final progress update on connect failure

f30a872

To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239

meslubi2021 pushed a commit to Unity-Curl/curl that referenced this pull request Jul 22, 2024

multi: do a final progress update on connect failure

7d88b9c

To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239

meslubi2021 pushed a commit to Unity-Curl/curl that referenced this pull request Jul 22, 2024

multi: do a final progress update on connect failure

5dd0a5a

To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239

meslubi2021 pushed a commit to Unity-Curl/curl that referenced this pull request Jul 22, 2024

multi: do a final progress update on connect failure

f808889

To fix timing metric etc Co-authored-by: Justin Maggard Fixes curl#14204 Closes curl#14239

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multi: fix timing metrics on connect failure #14204

multi: fix timing metrics on connect failure #14204

jmaggard10 commented Jul 16, 2024

dfandrich commented Jul 18, 2024

dfandrich commented Jul 18, 2024

bagder left a comment

jmaggard10 commented Jul 19, 2024

bagder commented Jul 19, 2024

bagder commented Jul 19, 2024

jmaggard10 commented Jul 19, 2024

multi: fix timing metrics on connect failure #14204

multi: fix timing metrics on connect failure #14204

Conversation

jmaggard10 commented Jul 16, 2024

dfandrich commented Jul 18, 2024

Generated by Testclutch

dfandrich commented Jul 18, 2024

bagder left a comment

Choose a reason for hiding this comment

jmaggard10 commented Jul 19, 2024

bagder commented Jul 19, 2024

bagder commented Jul 19, 2024

jmaggard10 commented Jul 19, 2024