-
Notifications
You must be signed in to change notification settings - Fork 350
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
fix: SessionExpired issue #1057
base: master
Are you sure you want to change the base?
Conversation
Introducing a SessionWrapper that is responsible for renewing the Neo4J driver and session in case of SessionExpired exeption.
Hey! Thanks for this. This could be a good solution, until the neo4j client libraries become more flexible with timeouts. Can you into potential performance impacts? |
Hi @AlmogBaku, thank you for putting this together. Couple of questions:
|
@ramonpetgrave64 - the performance impact is relatively minor since I only keep one instance and invalidate it upon failure.
I don't know about the
|
Ah, I meant can you provide a log trace showing where the crash happens? This way we might be able to identify if the issue is more likely to show up in certain parts of the code.
Can you redact out the sensitive parts? It would be helpful to see a failing log trace where the issue happens, and a successful log trace to show that the issue is solved. Just for additional reference, we implemented max-connection-lifetime here: #535 and it helped solve timeouts with our NLBs. If you haven't already, I'd definitely recommend setting it to something less than 350s if you know that the connection is going over an AWS NLB (I don't know what Aura uses for hosting). I'd also be very interested in seeing the session expired crash even with this setting. To summarize, the logic table I'd want to know is
Transient crashes are frustrating and I definitely appreciate your help; I'm asking all these questions to gather data before deciding to own additional complexity (imo this kind of thing really should be handled by the neo4j driver :p) |
|
Hi @AlmogBaku, please send the log over, or if that is not possible then please reply back with what exact cartography function was running when it crashed (e.g. was it running load() in EC2 instances, or load() in IAM, or...?). It'd also be helpful to know if it always crashes in that same spot or if there are multiple places (if you have multiple crash dumps, then great, send them all if you can!). I'd really like to know you're seeing this crash as a result of session.run() or write_transaction(). |
Thanks @AlmogBaku for sharing the log, this is helpful. I'd like a bit of time to play with this a bit before merging. Thanks for your paitience. cc: @ramonpetgrave64 @mpurusottamc FYI. @harshagw just wondering, how difficult would it be to combine the neo4j session factory that you wrote with the session wrapper shown in this PR? |
any update? |
Introducing a SessionWrapper that is responsible for renewing the Neo4J driver and session in case of SessionExpired exception.
fixes #987