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

Switch default value of Neo4j open session in view #20012

Closed
michael-simons opened this issue Feb 3, 2020 · 3 comments
Closed

Switch default value of Neo4j open session in view #20012

michael-simons opened this issue Feb 3, 2020 · 3 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@michael-simons
Copy link
Contributor

michael-simons commented Feb 3, 2020

The current default of "open session in view" for Spring Data Neo4j hurts performance in cluster scenarios.

By opening the session on the request, the session will be a general purpose session ("read write") and always go to the leader of a cluster, leaving all followers and read replicas unused. I have written down an analysis of the problem here Spring Data Neo4j, Neo4j-OGM and OSIV.

Neo4j-OGM doesn't have the issue of lazy loading and keeping the session up and running through the request only helps caching inside OGM.

If such a change is unappropriated for 2.3, I would like to see a better warning, for example

logger.warn("spring.data.neo4j.open-in-view is enabled by default."
    + "Therefore, database queries may be performed during view "
    + "rendering. It also prevent's chosing anything else than the leader in a cluster scenario. Explicitly configure "
   + "spring.data.neo4j.open-in-view to disable this warning");

Or, the best might even check if one or more bolt+routing respectively neo4j:// Uris have been configured and than according to that, choose a sane default.

So, I basically see three options:

  • Change the warning to be more drastic or
  • Change the default or
  • Change the default conditionally when there's one or more bolt+routing:// or neo4j:// uris or more than one bolt:// URI.

I could implement each one of them, but wanted to hear your opinion on that.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 3, 2020
@michael-simons michael-simons changed the title Change the default of "open session in view" to false for Neo4j. Open session in view is problematic with Neo4j. Feb 3, 2020
@snicoll
Copy link
Member

snicoll commented Feb 3, 2020

@michael-simons thanks for the report and such a change for the reason you've described seems appropriate to me.

I don't really have an opinion besides the fact that the last option makes it harder to resonate what the default value of the flag is. If we keep a boolean flag, I'd rather keep a straight true/false rather than true if the transport is configured in a certain way.

My vote would go for #2. Flagging for team attention to see what the rest of the team thinks.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Feb 3, 2020
@michael-simons
Copy link
Contributor Author

Completely agree with you on this one.

@wilkinsona
Copy link
Member

wilkinsona commented Feb 3, 2020

Neo4j-OGM doesn't have the issue of lazy loading

In that case, +1 for changing the default. The pattern's only really of value when it reduces the learning curve of lazy loading problems. Without that problem to solve, it feels like a bad default to me.

@snicoll snicoll changed the title Open session in view is problematic with Neo4j. Switch default value of Neo4j open session in view Feb 3, 2020
@snicoll snicoll added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Feb 3, 2020
@snicoll snicoll added this to the 2.3.x milestone Feb 3, 2020
@snicoll snicoll self-assigned this Feb 10, 2020
@snicoll snicoll modified the milestones: 2.3.x, 2.3.0.M2 Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants