Skip to content

Conversation

@Trott
Copy link
Member

@TrottTrott commented Apr 30, 2016

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

process src test

Description of change

Add O_NOATIME fs flag on Linux.

WIP. Do not land (yet). (Running the test first everywhere to make sure results are as expected on all the Linux hosts.)

@TrottTrott added wip Issues and PRs that are still a work in progress. process Issues and PRs related to the process subsystem. labels Apr 30, 2016
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Apr 30, 2016
Add O_NOATIME flag on Linux for use with `fs.open()`. PR-URL: nodejs#6492Fixes: nodejs#2182
@TrottTrott removed the wip Issues and PRs that are still a work in progress. label Apr 30, 2016
@Trott
Copy link
MemberAuthor

Removed in-progress label. Ready for review.

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

@jasnelljasnell added wip Issues and PRs that are still a work in progress. and removed wip Issues and PRs that are still a work in progress. labels May 1, 2016
@jasnell
Copy link
Member

ha! saw the in progress comment in the PR description, added the label, then scanned lower ;-)

would it be possible to add a test that uses the constant? it's always concerned me that these things aren't documented that well :-(

@addaleax
Copy link
Member

would it be possible to add a test that uses the constant?

Unfortunately not in a reliable way, the flag is only advisory to the OS/the filesystem.

@jasnell
Copy link
Member

jasnell commented May 1, 2016

Didn't think so but didn't hurt to ask ;). Probably the only objection I
can think of on this is that constants is still not documented properly.

It's likely too late to do so now but ideally these kinds of constants
would be exposed via the module in which they are relevant (i.e. fs related
constants on the fs module, crypto related constants on the crypto module).
Add in proper documentation for them, then deprecate require('constants')
entirely (keeping the internal process.binding('constants') tho. Ah well.
On Apr 30, 2016 7:24 PM, "Anna Henningsen" [email protected] wrote:

would it be possible to add a test that uses the constant?

Unfortunately not in a reliable way, the flag is only advisory to the
OS/the filesystem.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#6492 (comment)

@bnoordhuis
Copy link
Member

LGTM

@jbergstroem
Copy link
Member

jbergstroem commented May 1, 2016

O_NOATIME arrived in linux 2.6.8 it seems. Is there a (supported) Linux distribution that we should care about? I couldn't find anything.

Edit: Some more pointless googling, treat my comment as noise.

@bnoordhuis
Copy link
Member

2.6.18 (RHEL 5) is the baseline, it was 2.6.9 (RHEL 4) before that.

@Trott
Copy link
MemberAuthor

Trott commented May 3, 2016

@jasnell
Copy link
Member

Related: #6534

@evanlucas
Copy link
Contributor

should constants be the subsystem?

@jasnell
Copy link
Member

src: would likely be more appropriate.

@jasnelljasnell added c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version. labels May 3, 2016
@jasnell
Copy link
Member

LGTM

Trott added a commit to Trott/io.js that referenced this pull request May 4, 2016
Add O_NOATIME flag on Linux for use with `fs.open()`. PR-URL: nodejs#6492Fixes: nodejs#2182 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@Trott
Copy link
MemberAuthor

Trott commented May 4, 2016

Landed in 52f85be

@TrottTrott closed this May 4, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
Add O_NOATIME flag on Linux for use with `fs.open()`. PR-URL: #6492Fixes: #2182 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
evanlucas added a commit that referenced this pull request May 17, 2016
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna Henningsen) [#6511](#6511) - **child_process**: use /system/bin/sh on android (Ben Noordhuis) [#6745](#6745) - **deps**: - upgrade npm to 3.8.9 (Rebecca Turner) [#6664](#6664) - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh) [#6572](#6572) - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé) [#6796](#6796) - Intl: ICU 57 bump (Steven R. Loomis) [#6088](#6088) - **repl**: - copying tabs shouldn't trigger completion (Eugene Obrezkov) [#5958](#5958) - exports `Recoverable` (Blake Embrey) [#3488](#3488) - **src**: add O_NOATIME constant (Rich Trott) [#6492](#6492) - **src,module**: add --preserve-symlinks command line flag (James M Snell) [#6537](#6537) - **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen) [#6683](#6683)
evanlucas added a commit that referenced this pull request May 17, 2016
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna Henningsen) [#6511](#6511) - **child_process**: use /system/bin/sh on android (Ben Noordhuis) [#6745](#6745) - **deps**: - upgrade npm to 3.8.9 (Rebecca Turner) [#6664](#6664) - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh) [#6572](#6572) - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé) [#6796](#6796) - Intl: ICU 57 bump (Steven R. Loomis) [#6088](#6088) - **repl**: - copying tabs shouldn't trigger completion (Eugene Obrezkov) [#5958](#5958) - exports `Recoverable` (Blake Embrey) [#3488](#3488) - **src**: add O_NOATIME constant (Rich Trott) [#6492](#6492) - **src,module**: add --preserve-symlinks command line flag (James M Snell) [#6537](#6537) - **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen) [#6683](#6683) As of this release the 6.X line now includes 64-bit binaries for Linux on Power Systems running in big endian mode in addition to the existing 64-bit binaries for running in little endian mode. PR-URL: #6810
evanlucas added a commit that referenced this pull request May 17, 2016
- **buffer**: fix lastIndexOf and indexOf in various edge cases (Anna Henningsen) [#6511](#6511) - **child_process**: use /system/bin/sh on android (Ben Noordhuis) [#6745](#6745) - **deps**: - upgrade npm to 3.8.9 (Rebecca Turner) [#6664](#6664) - upgrade to V8 5.0.71.47 (Ali Ijaz Sheikh) [#6572](#6572) - upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé) [#6796](#6796) - Intl: ICU 57 bump (Steven R. Loomis) [#6088](#6088) - **repl**: - copying tabs shouldn't trigger completion (Eugene Obrezkov) [#5958](#5958) - exports `Recoverable` (Blake Embrey) [#3488](#3488) - **src**: add O_NOATIME constant (Rich Trott) [#6492](#6492) - **src,module**: add --preserve-symlinks command line flag (James M Snell) [#6537](#6537) - **util**: adhere to `noDeprecation` set at runtime (Anna Henningsen) [#6683](#6683) As of this release the 6.X line now includes 64-bit binaries for Linux on Power Systems running in big endian mode in addition to the existing 64-bit binaries for running in little endian mode. PR-URL: #6810
@TrottTrott deleted the noatime branch January 13, 2022 22:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.processIssues and PRs related to the process subsystem.semver-minorPRs that contain new features and should be released in the next minor version.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@Trott@jasnell@addaleax@bnoordhuis@jbergstroem@evanlucas@MylesBorins@nodejs-github-bot