node-neo4j icon indicating copy to clipboard operation
node-neo4j copied to clipboard

Support specifying max execution time?

Open aseemk opened this issue 13 years ago • 13 comments

From Neo4j 1.6's changelog:

o Added request timeout, controlled with org.neo4j.server.webserver.limit.executiontime (disabled by default). Request header (max-execution-time) can shorten it if specified.

Might be nice to add support into our library to specify this on a request basis. We probably won't make use of it yet though, so chime in if you think you might. (And pull requests always welcome. =D)

aseemk avatar Jan 29 '12 19:01 aseemk

Good to know - not sure if I can think of a reason I'd use it now!

On Sun, Jan 29, 2012 at 2:34 PM, Aseem Kishore [email protected] wrote:

From Neo4j 1.6's changelog:

o Added request timeout, controlled with org.neo4j.server.webserver.limit.executiontime (disabled by default). Request header (max-execution-time) can shorten it if specified.

Might be nice to add support into our library to specify this on a request basis. We probably won't make use of it yet though, so chime in if you think you might. (And pull requests always welcome. =D)


Reply to this email directly or view it on GitHub: https://github.com/thingdom/node-neo4j/issues/13

jeremyis avatar Jan 29 '12 22:01 jeremyis

I'd like to se this implemented. Even just allowing me to pass the request.timeout setting into the GraphDatabase constructor would be helpful.

lookfirst avatar Feb 20 '13 10:02 lookfirst

node-neo4j doesn't call any constructors since it's using REST. I'm curious whether the request header actually works--hadn't heard of it until now. I think I'll try it out real quick.

freeeve avatar Feb 21 '13 05:02 freeeve

@wfreeman Sorry, I don't understand your response, maybe I wasn't clear.

https://github.com/mikeal/request#requestoptions-callback

timeout - Integer containing the number of milliseconds to wait for a request to respond before aborting the request

Thus, I'd like...

exports.neo = neo = new neo4j.GraphDatabase(url: 'http://boo/', timeout: 10000)

Which would pass the timeout property to the underlying request

lookfirst avatar Feb 21 '13 05:02 lookfirst

Sorry, I thought you were talking about the server-side constructor, which didn't make sense. You're saying you'd like the client side request to timeout even if the server keeps crunching?

freeeve avatar Feb 21 '13 05:02 freeeve

Yes. In our case, we have a limit for the length we are willing to wait for Neo to respond. Right now, it is great that it is so fast, but if there is ever a hiccup in things and Neo locks up, we just want to kill the connection after a timeout. The reason is that we aren't always 100% dependent on the data from Neo. Our app keeps on working, it just has less useful information in the response. When running on Heroku, we have to service requests as quickly as possible or we get the dreaded H12 errors. So, having a solution to cut Neo off at the knees when it isn't behaving properly would be nice. Also, if the code is allowing one to pass in arguments to request, might as well allow them all as it is impossible to predict what users will want in the future (such as this requirement).

lookfirst avatar Feb 21 '13 06:02 lookfirst

I'm doing some issue cleanup, and haven't heard any more requests for this in a long time, so going ahead and closing this as "won't implement". Feel free to speak up if you disagree!

aseemk avatar Nov 07 '14 10:11 aseemk

Not having requests doesn't mean that it shouldn't be implemented.

lookfirst avatar Nov 07 '14 17:11 lookfirst

Sorry @lookfirst, missed your response. Okay, reopening then. This is easy to add; I'll try to include it in my v2 work in pull #145.

aseemk avatar Jan 30 '15 18:01 aseemk

I'm curious though: do you still want a client-side timeout if we can specify a server-side one? The server-side one will give you a guarantee that the db state didn't change, and you'll get back Neo's proper GuardTimeoutException. If we do a client-side one, it'll just look like an aborted request.

aseemk avatar Jan 30 '15 18:01 aseemk

I suppose the flip-side argument is that you can specify the server-side timeout in the Neo4j configs (as we do and as is recommended, maybe even the default), so perhaps the option is more useful for a client-side timeout. I'm not sure.

aseemk avatar Jan 30 '15 18:01 aseemk

@aseemk I haven't used this project in ages. I've offloaded my opinions at this point. Sorry. =(

lookfirst avatar Feb 03 '15 18:02 lookfirst

As noted in issue #177, it's now possible to achieve this in node-neo4j v2 since you can specify custom headers. =)

This issue will close the moment the v2 branch is merged to master, but you can use it today:

Issue: #143 / PR: #145 Docs: https://github.com/thingdom/node-neo4j/tree/v2#readme

aseemk avatar Oct 12 '15 16:10 aseemk