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

Comments

@mhdawson
Copy link
Member

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

  • 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 mhdawson self-assigned this Feb 4, 2016
@mhdawson mhdawson added the master label Feb 4, 2016
@mscdex mscdex added the fs label Feb 5, 2016
@mhdawson

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Feb 11, 2016

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 added 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 added 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 added 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 added 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 added 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

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2016

Some progress here: libuv/libuv#776

@mhdawson

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2016

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

@mhdawson mhdawson referenced this issue Mar 28, 2016
0 of 2 tasks complete
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

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2016

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

evanlucas added 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 added 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

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2016

@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

This comment has been minimized.

Copy link
Member

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 ppc labels Nov 16, 2016
cjihrig added a commit that referenced this issue Feb 9, 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>
italoacasas added a commit that referenced this issue Feb 13, 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>
italoacasas added 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 added 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

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

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

@gireeshpunathil

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

yes please. This is complete.

@gibfahn gibfahn closed this Mar 29, 2017
@Trott Trott referenced this issue Apr 21, 2017
2 of 2 tasks complete
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 added 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 added 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 added 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 added 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 added 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 added 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 added 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 added 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 added 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 added 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 added 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 added 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 added 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
Projects
None yet
5 participants
You can’t perform that action at this time.