Support specifying max execution time?

[aseemk]

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)

[jeremyis]

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:
#13

[lookfirst]

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

[freeeve]

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.

[lookfirst]

@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

[freeeve]

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?

[lookfirst]

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

[aseemk]

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!

[lookfirst]

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

[aseemk]

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]

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]

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.

[lookfirst]

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

[aseemk]

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