[v18.x] Temporarily revert libuv #50036

richardlau · 2023-10-04T12:17:15Z

Temporarily revert recent libuv updates (1.46.0 and 1.45.0) in Node.js 18 to fix regressions:

Also #49911 which is a separate bug that was exposed by the performance profile change that IO_URING introduced (for this particular one I've already landed the Node.js fix on the staging branch but it demonstrates the risk that drastic performance changes can have on LTS release lines).

While the IO_URING issues can be worked around by setting UV_USE_IO_URING=0 in the environment, the Windows FS regression cannot. Upstream libuv contains fixes for the Windows regression and IO_URING fixes that are not yet in a libuv release:

I think it's too early to have IO_URING enabled by default in LTS Node.js. Were it just the IO_URING issues I might propose patching libuv to disable it by default in Node.js 18 instead of reverting the updates.

Even if a new libuv version was to be made available now we normally would not land that in LTS until it has been released in a current release for two weeks.

cc @nodejs/lts @nodejs/releasers

This reverts commit ea23870.

This reverts commit 7dc731d.

This reverts commit 88855e0.

This reverts commit fb2b80f.

richardlau · 2023-10-04T12:22:00Z

For transparency, reverting libuv 1.45.0 and 1.46.0 would undo the changes noted in

nodejs-github-bot · 2023-10-04T12:25:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54522/

mhdawson

Rubber stamp LGTM

targos · 2023-10-04T14:22:57Z

What do you think is the risk to break users who might depend on the new libuv versions being present?

mhdawson · 2023-10-04T14:45:36Z

@targos are you thinking about native addons or something else in terms of dependencies on newer versions of libuv?

targos · 2023-10-04T15:00:32Z

Nothing in particular. Could be native addons. Could be bug fixes that affect specific workloads and are now depended upon.
I think disabling IO_URING should be safe, but we need to be careful with full reverts. The vast majority of production deployments are not on Windows.

richardlau · 2023-10-04T15:03:14Z

What do you think is the risk to break users who might depend on the new libuv versions being present?

Low? Of the notable changes in:

deps: upgrade to libuv 1.45.0 #48078
- IO_URING (undoing this is part of the reasoning for this PR)
- new metrics API is unused in Node.js. I suppose a native addon might use it but we're talking one Node.js 18 version (18.18.0).
- I don't know about the thread pool change nor timer reordering changes
- The fixes are interesting -- the cgroupsv2 fix was requested in the 18.18.0 proposal (v18.18.0 proposal #49220 (comment)). I guess the question is balancing the Windows regression in (Windows) FS can not handle certain characters in file name #48673 against the fixes made in libuv 1.45.0.
deps: upgrade to libuv 1.46.0 #48618
- The use WTF-8 change on Windows is the cause of (Windows) FS can not handle certain characters in file name #48673 so again part of the reason for this revert PR.
- IO_URING changes would be ignorable since the revert would remove IO_URING completely.
- I don't know if the timer fix here fixes an issue that was exposed through Node.js.

Node.js 18 has had regressions for the last three releases

18.18.0 as documented in the description of this PR
18.17.1 (security release) and 18.17.0 both had the URL parsing regression url.parse regression in 18.17+ #49330

Reverting the libuv change would be a fairly quick way of fixing the regressions in Node.js 18.18.0. I can volunteer to do a quick Node.js 18.18.1 release containing this PR -- I would not have time to volunteer to do a full Node.js 18 release this month.

If people would rather we did not temporarily revert libuv in Node.js 18 please request changes on this PR. I'm happy to go with whatever consensus @nodejs/lts comes to.

santigimeno

Seems reasonable to me. Will try to cut a new libuv release with those fixes soonish.

nodejs-github-bot · 2023-10-04T16:28:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54535/

arsnyder16 · 2023-10-04T16:45:01Z

Are these issues also present in node 20? Is there a plan to revert libuv in 20 as well?

I am just wondering what the options are for cgroup v2 and LTS

richardlau · 2023-10-04T16:48:54Z

Are these issues also present in node 20? Is there a plan to revert libuv in 20 as well?

I am just wondering what the options are for cgroup v2 and LTS

Currently no plans to revert in Node.js 20 -- the libuv updates have been in eight Node.js 20 releases (starting with 20.3.0 in June).

This reverts commit ea23870. PR-URL: #50036 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruy Adorno <ruyadorno@google.com>

This reverts commit 7dc731d. PR-URL: #50036 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruy Adorno <ruyadorno@google.com>

This reverts commit 88855e0. PR-URL: #50036 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruy Adorno <ruyadorno@google.com>

This reverts commit fb2b80f. PR-URL: #50036 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruy Adorno <ruyadorno@google.com>

richardlau · 2023-10-06T16:03:06Z

Landed in 7a3e1ff...14ece2c.

nodejs/node#50036

* chore: bump node in DEPS to v18.18.1 * Revert "deps: upgrade to libuv 1.46.0" nodejs/node#50036 * chore: fixup patch indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* chore: bump node to v18.18.1 (main) (#40174) * chore: bump node in DEPS to v18.18.1 * Revert "deps: upgrade to libuv 1.46.0" nodejs/node#50036 * chore: fixup patch indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> * chore: bump node to v18.18.2 (main) (#40205) * chore: bump node in DEPS to v18.18.2 * chore: update patches * deps: update nghttp2 to 1.55.0 nodejs/node#48746 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

* chore: bump node in DEPS to v18.18.1 * Revert "deps: upgrade to libuv 1.46.0" nodejs/node#50036 * chore: fixup patch indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

richardlau added 4 commits October 3, 2023 15:44

Revert "deps: add missing thread-common.c in uv.gyp"

64fad26

This reverts commit ea23870.

Revert "build: sync libuv header change"

db19319

This reverts commit 7dc731d.

Revert "deps: upgrade to libuv 1.46.0"

c9f4f90

This reverts commit 88855e0.

Revert "deps: upgrade to libuv 1.45.0"

4ec567f

This reverts commit fb2b80f.

richardlau added libuv Issues and PRs related to the libuv dependency or the uv binding. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Oct 4, 2023

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 4, 2023

richardlau 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 Oct 4, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 4, 2023

mhdawson approved these changes Oct 4, 2023

View reviewed changes

santigimeno approved these changes Oct 4, 2023

View reviewed changes

targos approved these changes Oct 4, 2023

View reviewed changes

ruyadorno approved these changes Oct 4, 2023

View reviewed changes

ruyadorno mentioned this pull request Oct 4, 2023

[v18.x backport] deps: upgrade to libuv 1.46.0 #48618 #49591

Closed

This was referenced Oct 5, 2023

CI Reliability 2023-10-05 nodejs/reliability#684

Open

CI Reliability 2023-10-06 nodejs/reliability#685

Open

richardlau closed this Oct 6, 2023

richardlau deleted the v18.x-revert-libuv branch October 6, 2023 16:03

This was referenced Oct 6, 2023

v18.18.1 proposal #50065

Closed

v18.18.1 proposal #50066

Merged

This was referenced Oct 7, 2023

CI Reliability 2023-10-07 nodejs/reliability#686

Open

CI Reliability 2023-10-08 nodejs/reliability#687

Open

esetnik mentioned this pull request Oct 10, 2023

cypress/included images "text file busy" with node 20 cypress-io/cypress#27707

Open

codebytere added a commit to electron/electron that referenced this pull request Oct 11, 2023

Revert "deps: upgrade to libuv 1.46.0"

c5a03b3

nodejs/node#50036

codebytere added a commit to electron/electron that referenced this pull request Oct 11, 2023

Revert "deps: upgrade to libuv 1.46.0"

e069ec6

nodejs/node#50036

rti mentioned this pull request Jan 30, 2024

[Bug] Docker build creates yarn error with esbuild rti/gbnc#16

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v18.x] Temporarily revert libuv #50036

[v18.x] Temporarily revert libuv #50036

richardlau commented Oct 4, 2023 •

edited

richardlau commented Oct 4, 2023

nodejs-github-bot commented Oct 4, 2023

mhdawson left a comment

targos commented Oct 4, 2023

mhdawson commented Oct 4, 2023

targos commented Oct 4, 2023

richardlau commented Oct 4, 2023

santigimeno left a comment

nodejs-github-bot commented Oct 4, 2023

arsnyder16 commented Oct 4, 2023 •

edited

richardlau commented Oct 4, 2023

richardlau commented Oct 6, 2023

[v18.x] Temporarily revert libuv #50036

[v18.x] Temporarily revert libuv #50036

Conversation

richardlau commented Oct 4, 2023 • edited

richardlau commented Oct 4, 2023

nodejs-github-bot commented Oct 4, 2023

mhdawson left a comment

Choose a reason for hiding this comment

targos commented Oct 4, 2023

mhdawson commented Oct 4, 2023

targos commented Oct 4, 2023

richardlau commented Oct 4, 2023

santigimeno left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 4, 2023

arsnyder16 commented Oct 4, 2023 • edited

richardlau commented Oct 4, 2023

richardlau commented Oct 6, 2023

richardlau commented Oct 4, 2023 •

edited

arsnyder16 commented Oct 4, 2023 •

edited