You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "nickva (via GitHub)" <gi...@apache.org> on 2023/03/29 19:32:50 UTC
[GitHub] [couchdb] nickva opened a new pull request, #4505: Treat javascript internal errors as fatal
nickva opened a new pull request, #4505:
URL: https://github.com/apache/couchdb/pull/4505
Spidermonkey sometimes throws an `InternalError` when exceeding memory limits, when normally we'd expect it to crash or exit with a non-0 exit code. Because we trap exceptions, and continue emitting rows, it is possible for users views to randomly miss indexed rows based on whether GC had run or not, other internal runtime state which may have been consuming more or less memory until that time.
To prevent the view continuing processing documents, and randomly dropping emitted rows, depending on memory pressure in the JS runtime at the time, choose to treat Internal errors as fatal.
After an InternalError is raised we expect the process to exit just like it would during OOM.
Add a test to assert this happens.
Fix https://github.com/apache/couchdb/issues/4504
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva merged pull request #4505: Treat javascript internal errors as fatal
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva merged PR #4505:
URL: https://github.com/apache/couchdb/pull/4505
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] davisp commented on a diff in pull request #4505: Treat javascript internal errors as fatal
Posted by "davisp (via GitHub)" <gi...@apache.org>.
davisp commented on code in PR #4505:
URL: https://github.com/apache/couchdb/pull/4505#discussion_r1152432758
##########
share/server/loop.js:
##########
@@ -133,6 +133,9 @@ var Loop = function() {
} else if (e.error && e.reason) {
// compatibility with old error format
respond(["error", e.error, e.reason]);
+ } else if (e.name == "InternalError") {
+ respond(["error", e.name, e.message]);
+ quit(-1);
Review Comment:
I think you probably want to move this conditional up to the second if the if/else chain or you run the risk of this being caught on line 133.
##########
share/server/loop.js:
##########
@@ -133,6 +133,9 @@ var Loop = function() {
} else if (e.error && e.reason) {
// compatibility with old error format
respond(["error", e.error, e.reason]);
+ } else if (e.name == "InternalError") {
+ respond(["error", e.name, e.message]);
+ quit(-1);
Review Comment:
Also, is this belt and suspended bracers? Becauses the change in view.js makes me think this should already be caught be the first conditional checking for "fatal".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4505: Treat javascript internal errors as fatal
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #4505:
URL: https://github.com/apache/couchdb/pull/4505#discussion_r1152438504
##########
share/server/loop.js:
##########
@@ -133,6 +133,9 @@ var Loop = function() {
} else if (e.error && e.reason) {
// compatibility with old error format
respond(["error", e.error, e.reason]);
+ } else if (e.name == "InternalError") {
+ respond(["error", e.name, e.message]);
+ quit(-1);
Review Comment:
Good call on moving it up. It's belt and suspenders if we crash outside of the view function itself for instance when serializing the data to be returned back to the user.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org