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.

Sign up for GitHub

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

Jump to bottom

process: add O_NOATIME constant #6492

Closed
wants to merge 1 commit into from
Closed

process: add O_NOATIME constant #6492

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Apr 30, 2016 •
edited
Loading

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.)

@Trott Trott 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-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 30, 2016
@Trott
process: add O_NOATIME constant
cf4c83c 
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: nodejs#6492
Fixes: nodejs#2182
@Trott Trott removed the wip Issues and PRs that are still a work in progress. label Apr 30, 2016
@Trott
Copy link
Member Author

Trott commented Apr 30, 2016

Removed in-progress label. Ready for review.

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

@Trott Trott mentioned this pull request Apr 30, 2016
Closed
@jasnell jasnell 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

jasnell commented May 1, 2016

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

addaleax commented May 1, 2016

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 •
edited
Loading

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" notifications@github.com 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 •
edited
Loading

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.

@jasnell jasnell mentioned this pull request May 1, 2016
Closed
1 task
@Trott
Copy link
Member Author

Trott commented May 3, 2016

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

@jasnell
Copy link
Member

jasnell commented May 3, 2016

Related: #6534

@evanlucas
Copy link
Contributor

should constants be the subsystem?

@jasnell
Copy link
Member

jasnell commented May 3, 2016

src: would likely be more appropriate.

@jasnell jasnell 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

jasnell commented May 3, 2016

LGTM

Trott added a commit to Trott/io.js that referenced this pull request May 4, 2016
@Trott
src: add O_NOATIME constant
52f85be 
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: nodejs#6492
Fixes: nodejs#2182
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@Trott
Copy link
Member Author

Trott commented May 4, 2016

Landed in 52f85be

@Trott Trott closed this May 4, 2016
@thefourtheye thefourtheye mentioned this pull request May 6, 2016
Closed
3 tasks
evanlucas pushed a commit that referenced this pull request May 17, 2016
@Trott @evanlucas
src: add O_NOATIME constant
37736f4 
Add O_NOATIME flag on Linux for use with `fs.open()`.

PR-URL: #6492
Fixes: #2182
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@evanlucas evanlucas mentioned this pull request May 17, 2016
Merged
evanlucas added a commit that referenced this pull request May 17, 2016
@evanlucas
2016-05-17, Version 6.2.0 (Stable)
9fab6aa 
- **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
@evanlucas
2016-05-17, Version 6.2.0 (Stable)
a4705f6 
- **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
@evanlucas
2016-05-17, Version 6.2.0 (Stable)
26120e2 
- **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
@Trott Trott deleted the noatime branch January 13, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Trott @jasnell @addaleax @bnoordhuis @jbergstroem @evanlucas @MylesBorins @nodejs-github-bot