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

AIX fs watch improvements #5085

Closed
mhdawson opened this issue Feb 4, 2016 · 18 comments
Closed

AIX fs watch improvements #5085

mhdawson opened this issue Feb 4, 2016 · 18 comments
Assignees
Labels
aix Issues and PRs related to the AIX platform. fs Issues and PRs related to the fs subsystem / file system. ppc Issues and PRs related to the Power architecture.

Comments

@mhdawson
Copy link
Member

mhdawson commented Feb 4, 2016

Currently the AIX fs watch implementation requires some prior configuration and is not necessarily straight forward to use on AIX.

This issue is to track work to improve the implementation or better document to make it easier to use.

Currently the following tests fail in the CI due to fs watch not being configured:

https://ci.nodejs.org/job/node-test-commit-aix/33/nodes=aix61-ppc64/console

not ok 4 test-async-wrap-check-providers.js
# fs.js:1329
#     throw error;
#     ^
# 
# Error: watch /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/parallel/test-async-wrap-check-providers.js ENOSYS
#     at exports._errnoException (util.js:859:11)
#     at FSWatcher.start (fs.js:1327:19)
#     at Object.fs.watch (fs.js:1355:11)
#     at Object. (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/parallel/test-async-wrap-check-providers.js:38:4)
#     at Module._compile (module.js:417:34)
#     at Object.Module._extensions..js (module.js:426:10)
#     at Module.load (module.js:357:32)
#     at Function.Module._load (module.js:314:12)
#     at Function.Module.runMain (module.js:451:10)
#     at startup (node.js:139:18)
  ---
  duration_ms: 0.448

not ok 4 test-async-wrap-check-providers.js
# fs.js:1329
#     throw error;
#     ^
# 
# Error: watch /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/parallel/test-async-wrap-check-providers.js ENOSYS
#     at exports._errnoException (util.js:859:11)
#     at FSWatcher.start (fs.js:1327:19)
#     at Object.fs.watch (fs.js:1355:11)
#     at Object. (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/parallel/test-async-wrap-check-providers.js:38:4)
#     at Module._compile (module.js:417:34)
#     at Object.Module._extensions..js (module.js:426:10)
#     at Module.load (module.js:357:32)
#     at Function.Module._load (module.js:314:12)
#     at Function.Module.runMain (module.js:451:10)
#     at startup (node.js:139:18)
  ---
  duration_ms: 0.448

not ok 980 test-fs-watch.js
# assert.js:324
#     throw actual;
#     ^
# 
# Error: watch /home/iojs/node-tmp/tmp.0/watch.txt ENOSYS
#     at exports._errnoException (util.js:859:11)
#     at FSWatcher.start (fs.js:1327:19)
#     at Object.fs.watch (fs.js:1355:11)
#     at /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/sequential/test-fs-watch.js:41:24
#     at Function._throws (assert.js:306:5)
#     at Function.assert.doesNotThrow (assert.js:337:11)
#     at Object. (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/sequential/test-fs-watch.js:39:8)
#     at Module._compile (module.js:417:34)
#     at Object.Module._extensions..js (module.js:426:10)
#     at Module.load (module.js:357:32)
  ---
  duration_ms: 1.147
  ...

The short term plan will be to exclude these to get the CI to a green state while we work in parallel on the resolution

@mhdawson mhdawson self-assigned this Feb 4, 2016
@mhdawson mhdawson added the master label Feb 4, 2016
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Feb 5, 2016
@mhdawson
Copy link
Member Author

PR to disabled the fs watch tests on AIX until we get this resolved. #5187

mhdawson added a commit to mhdawson/io.js that referenced this issue Feb 11, 2016
fs watch currently needs special configuration on AIX and we
want to improve under nodejs#5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX
@mhdawson
Copy link
Member Author

So we don't forget before we close this we need to revert #5187 including re-enabling the tests and undoing the change to test/parallel/test-async-wrap-check-providers.js

mhdawson added a commit that referenced this issue Feb 16, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue Feb 18, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg pushed a commit that referenced this issue Feb 18, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue Feb 18, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
stefanmb pushed a commit to stefanmb/node that referenced this issue Feb 23, 2016
fs watch currently needs special configuration on AIX and we
want to improve under nodejs#5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: nodejs#5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue Mar 2, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mhdawson
Copy link
Member Author

Some progress here: libuv/libuv#776

@mhdawson
Copy link
Member Author

A new failing test: test-fs-watch-encoding https://ci.nodejs.org/job/node-test-commit-aix/

mhdawson added a commit to mhdawson/io.js that referenced this issue Mar 28, 2016
As per nodejs#5085
exclude new test from AIX until we have fixes for
libuv for fs watching on AIX.  Excluding test
so AIX tests are green and we don't miss
other regressions
mhdawson added a commit that referenced this issue Mar 28, 2016
As per #5085
exclude new test from AIX until we have fixes for
libuv for fs watching on AIX.  Excluding test
so AIX tests are green and we don't miss
other regressions

PR-URL: #5937
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mhdawson
Copy link
Member Author

Adding so we remember to back this out once done: #5937

evanlucas pushed a commit that referenced this issue Mar 30, 2016
As per #5085
exclude new test from AIX until we have fixes for
libuv for fs watching on AIX.  Excluding test
so AIX tests are green and we don't miss
other regressions

PR-URL: #5937
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
evanlucas pushed a commit that referenced this issue Mar 31, 2016
As per #5085
exclude new test from AIX until we have fixes for
libuv for fs watching on AIX.  Excluding test
so AIX tests are green and we don't miss
other regressions

PR-URL: #5937
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mhdawson
Copy link
Member Author

@iwuzhere is making good progress on the libuv side existing fs tests are passing and he's just working on area that we saw was a gap even though there is no current test.

@jasnell
Copy link
Member

jasnell commented Jun 7, 2016

ping @mhdawson ...what's the status on this?

@mscdex mscdex removed the master label Oct 28, 2016
@mscdex mscdex added aix Issues and PRs related to the AIX platform. ppc Issues and PRs related to the Power architecture. labels Nov 16, 2016
italoacasas pushed a commit to italoacasas/node that referenced this issue Feb 14, 2017
On AIX, watch feature depends on AHAFS based Event infrastructure.
While in principle the watch use case is same across platforms, there
are subtle differences in the way AIX deals with this, with few
behavioral changes (external).

This commit addresses an assertion failure on folder watch, enabling the
AIX code for watch feature which was masked under a macro, open up
relevant test cases, skip tests which comes under the AIX limitation,
and make the document changes as appropriate.

Refs: nodejs#11094
Refs: nodejs#5085
PR-URL: nodejs#10085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
krydos pushed a commit to krydos/node that referenced this issue Feb 25, 2017
On AIX, watch feature depends on AHAFS based Event infrastructure.
While in principle the watch use case is same across platforms, there
are subtle differences in the way AIX deals with this, with few
behavioral changes (external).

This commit addresses an assertion failure on folder watch, enabling the
AIX code for watch feature which was masked under a macro, open up
relevant test cases, skip tests which comes under the AIX limitation,
and make the document changes as appropriate.

Refs: nodejs#11094
Refs: nodejs#5085
PR-URL: nodejs#10085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@gibfahn
Copy link
Member

gibfahn commented Mar 29, 2017

@gireeshpunathil is this complete? If so I'll close this out.

@gireeshpunathil
Copy link
Member

yes please. This is complete.

@gibfahn gibfahn closed this as completed Mar 29, 2017
Trott added a commit to Trott/io.js that referenced this issue Apr 21, 2017
nodejs#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.
jasnell pushed a commit that referenced this issue Apr 24, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
evanlucas pushed a commit that referenced this issue Apr 25, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Apr 27, 2017
The fs watch test was not run on AIX till now because of the known
issue, nodejs#5085. Now that it is
completed, this guard can be removed.
gibfahn pushed a commit that referenced this issue Apr 28, 2017
The fs watch test was not run on AIX till now because of the known
issue, #5085. Now that it is
completed, this guard can be removed.

PR-URL: #12687
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
evanlucas pushed a commit that referenced this issue May 1, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
evanlucas pushed a commit that referenced this issue May 2, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this issue May 15, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this issue May 16, 2017
On AIX, watch feature depends on AHAFS based Event infrastructure.
While in principle the watch use case is same across platforms, there
are subtle differences in the way AIX deals with this, with few
behavioral changes (external).

This commit addresses an assertion failure on folder watch, enabling the
AIX code for watch feature which was masked under a macro, open up
relevant test cases, skip tests which comes under the AIX limitation,
and make the document changes as appropriate.

Refs: #11094
Refs: #5085
PR-URL: #10085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
On AIX, watch feature depends on AHAFS based Event infrastructure.
While in principle the watch use case is same across platforms, there
are subtle differences in the way AIX deals with this, with few
behavioral changes (external).

This commit addresses an assertion failure on folder watch, enabling the
AIX code for watch feature which was masked under a macro, open up
relevant test cases, skip tests which comes under the AIX limitation,
and make the document changes as appropriate.

Refs: #11094
Refs: #5085
PR-URL: #10085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
On AIX, watch feature depends on AHAFS based Event infrastructure.
While in principle the watch use case is same across platforms, there
are subtle differences in the way AIX deals with this, with few
behavioral changes (external).

This commit addresses an assertion failure on folder watch, enabling the
AIX code for watch feature which was masked under a macro, open up
relevant test cases, skip tests which comes under the AIX limitation,
and make the document changes as appropriate.

Refs: #11094
Refs: #5085
PR-URL: #10085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: #12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this issue Jul 19, 2017
On AIX, watch feature depends on AHAFS based Event infrastructure.
While in principle the watch use case is same across platforms, there
are subtle differences in the way AIX deals with this, with few
behavioral changes (external).

This commit addresses an assertion failure on folder watch, enabling the
AIX code for watch feature which was masked under a macro, open up
relevant test cases, skip tests which comes under the AIX limitation,
and make the document changes as appropriate.

Refs: nodejs/node#11094
Refs: nodejs/node#5085
PR-URL: nodejs/node#10085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this issue Jul 19, 2017
nodejs/node#5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.

PR-URL: nodejs/node#12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aix Issues and PRs related to the AIX platform. fs Issues and PRs related to the fs subsystem / file system. ppc Issues and PRs related to the Power architecture.
Projects
None yet
Development

No branches or pull requests

5 participants