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

deps: update nghttp3 to 1.0.0 #47576

Closed
wants to merge 3 commits into from

Conversation

nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot commented Apr 16, 2023

This is an automated update of nghttp3 to 1.0.0.

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. dont-land-on-v14.x needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3. labels Apr 16, 2023
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

RSLGTM

Copy link
Member

@VoltrexKeyva VoltrexKeyva left a comment

Choose a reason for hiding this comment

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

RSLGTM

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 26, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 26, 2023
@nodejs-github-bot
Copy link
Collaborator Author

Commit Queue failed
- Loading data for nodejs/node/pull/47576
✔  Done loading data for nodejs/node/pull/47576
----------------------------------- PR info ------------------------------------
Title      deps: update nghttp3 to 0.10.0 (#47576)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     nodejs-github-bot:actions/tools-update-nghttp3 -> nodejs:main
Labels     needs-ci, quic, dependencies, dont-land-on-v14.x
Commits    1
 - deps: update nghttp3 to 0.10.0
Committers 1
 - Node.js GitHub Bot 
PR-URL: https://github.com/nodejs/node/pull/47576
Reviewed-By: Luigi Pinca 
Reviewed-By: Mohammed Keyvanzadeh 
Reviewed-By: Tobias Nießen 
Reviewed-By: Marco Ippolito 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/47576
Reviewed-By: Luigi Pinca 
Reviewed-By: Mohammed Keyvanzadeh 
Reviewed-By: Tobias Nießen 
Reviewed-By: Marco Ippolito 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 16 Apr 2023 00:24:33 GMT
   ✔  Approvals: 5
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/47576#pullrequestreview-1386703737
   ✔  - Mohammed Keyvanzadeh (@VoltrexKeyva): https://github.com/nodejs/node/pull/47576#pullrequestreview-1386816166
   ✔  - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/47576#pullrequestreview-1387101562
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/47576#pullrequestreview-1387635921
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/47576#pullrequestreview-1389053297
   ✔  Last GitHub CI successful
   ✘  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/4805669128

@targos targos added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 26, 2023
@nodejs-github-bot
Copy link
Collaborator Author

@nodejs-github-bot nodejs-github-bot changed the title deps: update nghttp3 to 0.10.0 deps: update nghttp3 to 0.11.0 Apr 30, 2023
@targos targos added commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. request-ci Add this label to start a Jenkins CI on a PR. labels May 1, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 1, 2023
@nodejs-github-bot
Copy link
Collaborator Author

targos pushed a commit that referenced this pull request May 15, 2023
PR-URL: #47992
Refs: #47576
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@marco-ippolito marco-ippolito added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels May 18, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2023
@nodejs-github-bot
Copy link
Collaborator Author

@nodejs-github-bot
Copy link
Collaborator Author

Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

Just making sure this doesn't land as-is

@fasenderos
Copy link
Contributor

fasenderos commented May 27, 2023

That's really strange. I can't explain why, cause locally never happens, but the job on 30th April that try to update ngtcp2 5e7d37e and nghttp3 69f8b22 create duplicated code.

nghttp3 on file deps/ngtcp2/nghttp3/lib/includes/nghttp3/nghttp3.h

/**
* @function
*
* `nghttp3_conn_is_drained` returns nonzero if
* `nghttp3_conn_shutdown` has been called, and there is no active
* remote streams. This function is for server use only.
*/
NGHTTP3_EXTERN int nghttp3_conn_is_drained(nghttp3_conn *conn);
/**
* @function
*
* `nghttp3_conn_is_drained` returns nonzero if
* `nghttp3_conn_shutdown` has been called, and there is no active
* remote streams. This function is for server use only.
*/
NGHTTP3_EXTERN int nghttp3_conn_is_drained(nghttp3_conn *conn);

ngtcp2 on file deps/ngtcp2/ngtcp2/lib/ngtcp2_cc.c (#47578)

cwnd_thres =
(target * (((t + cstat->smoothed_rtt) << 10) / NGTCP2_SECONDS)) >> 10;
if (cwnd_thres < cstat->cwnd) {
target = cstat->cwnd;
} else if (2 * cwnd_thres > 3 * cstat->cwnd) {
target = cstat->cwnd * 3 / 2;
} else {
target = cwnd_thres;
}
cwnd_thres =
(target * (((t + cstat->smoothed_rtt) << 10) / NGTCP2_SECONDS)) >> 10;
if (cwnd_thres < cstat->cwnd) {
target = cstat->cwnd;
} else if (2 * cwnd_thres > 3 * cstat->cwnd) {
target = cstat->cwnd * 3 / 2;
} else {
target = cwnd_thres;
}

Of course both duplicate codes never existed in the depencies repos

Last but not least the job that in theory should have skipped the update, because locally it actually skips the update, fix these differences with a second commit for the same version (on 7th May)

@nodejs-github-bot nodejs-github-bot changed the title deps: update nghttp3 to 0.11.0 deps: update nghttp3 to 0.12.0 Jun 11, 2023
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
PR-URL: #47992
Refs: #47576
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
PR-URL: nodejs#47992
Refs: nodejs#47576
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot changed the title deps: update nghttp3 to 0.12.0 deps: update nghttp3 to 0.13.0 Jul 16, 2023
@nodejs-github-bot nodejs-github-bot changed the title deps: update nghttp3 to 0.13.0 deps: update nghttp3 to 0.14.0 Aug 6, 2023
@nodejs-github-bot nodejs-github-bot changed the title deps: update nghttp3 to 0.14.0 deps: update nghttp3 to 0.15.0 Sep 3, 2023
@nodejs-github-bot nodejs-github-bot changed the title deps: update nghttp3 to 0.15.0 deps: update nghttp3 to 1.0.0 Oct 22, 2023
@mhdawson
Copy link
Member

@marco-ippolito wondering if you are still looking at this issue? I wonder if it might be at all related to the same branch being used by the updater each time. Have we tried closing this PR, deleting the branch and seeing if the problem re-occurs in the newly created PR?

@marco-ippolito
Copy link
Member

I totally missed this notification, I agree we should delete it and see what happens with the update. I had look at the code and tested on my machine and it was all working fine

@mhdawson
Copy link
Member

Closing and will delete branch, then we can see what happens

@mhdawson mhdawson closed this Oct 24, 2023
@mhdawson mhdawson deleted the actions/tools-update-nghttp3 branch October 24, 2023 20:06
@mhdawson
Copy link
Member

Branch deleted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dependencies Pull requests that update a dependency file. quic Issues and PRs related to the QUIC implementation / HTTP/3.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants