Session cache is cleared to often

[torstenkuhnhenne]

In OGM Release 3.2.15 there was a change that the session cache is cleared when a potential write query is executed (ExecuteQueryDelegates#query line 125). I use stored procedures to read data from neo4j and now the session cache is cleared after each call of the stored procedure. Which can lead to performance degeneration.

Expected Behavior

Clear the session cache only if necessary. Maybe it is possible to get the statistics back from the database as in the Neo4j Browser and use this information to decide wheter the cache must be cleared or not.

Current Behavior

Cache is cleared on every call of a stored procedure, even if it a stored procedure that only reads data.

Context

As I use stored procedures to read data from neo4j the session cache is now more or less useless for me because it is cleared to often.

Your Environment

• Java version:
• Neo4j version (The version of the database): 3.5.21
• OGM Version (The version of this software): 3.2.15
• OGM Transport used (One of Bolt, HTTP or embedded): Bolt
• Neo4j Java Driver version (in case of Bolt transport): 4.1.1
• Link to your project: ./.

Steps to Reproduce

1. Call a stored procedure in a read-only transaction
2. Session cache is cleared after each call

[michael-simons]

This is something we are not gonna role back. It has to much potential to lead to inconsistent data: https://jira.spring.io/browse/DATAGRAPH-1249.

I would rather introduce an additional overload for public <T> Iterable<T> query(Class<T> type, String cypher, Map<String, ?> parameters with a boolean to indicate you want read or write calls.

Would that work for you?

[torstenkuhnhenne]

The call is not directly via OGM. I have a SDN respository with a @Query annotation that calls the stored procedure, so the parameter does not help me.

I understand that you need to clear the cache and totally agree that it otherwise can lead to inconsistent data.

Is it an option to check the transaction mode and clear the cache if it is a read-write transaction and keep it if its a read-only transaction?

[michael-simons]

I see. No, in that specific use case (custom query for known entity classes) there's no propagation.

[michael-simons]

About how many custom queries are we talking? Would you be able to add a hint to them?

I'm thinking about something like this CALL /*+ OGM READ_ONLY */ dbms.functions() yield name.

I would rather not change the current SDN code to do this on some kind of heuristics.

The code path that is taken when you ask with. @Query for entities on SDN repositories doesn't allow to specify the mode… I would not only have to add an overload but also make sure SDN is aware which OGM version it uses (and we have already to many pieces of that).

I'll post a PR with the above functionality for you to try out. Looking forward to some feedback.

[torstenkuhnhenne]

I have to check it, I found 80 call statements but some of them are write-statements. For sure, it is possible to add a hint to them.

I will try out your changes and provide a feedback. My first thought was "looks like xml processing instructions in the 'good' old days" ;-)

[michael-simons]

Hehe… :) I was like "wow, now I'm adding query hints like I used to with Oracle."

[torstenkuhnhenne]

Ok, lets go on with the "like Oracle" approach ;-)

It works for me! I noticed that in line 127 there is also a session.clear() without the new check, maybe you should add it there. And also it is missing in validateQuery. Maybe the check should go to the method mayBeReadWrite?

Also I tried to integrate the new check in the existing regular expression. After some test with a colleague of mine we found a solution and I verified it with a JUnit 5 (with assertj) test:

import static org.assertj.core.api.Assertions.assertThat;

import java.util.regex.Pattern;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

/**
 * Test for new regular expression to determine write queries
 *
 * @author torsten.kuhnhenne
 */
class RegExTest {

    final String regex = "(?=^.*\\b(CREATE|MERGE|SET|DELETE|REMOVE|DROP|CALL)\\b)(?!^.*\\/\\*\\+ OGM READ_ONLY \\*\\/).*$";
    final Pattern pattern = Pattern.compile( this.regex, Pattern.MULTILINE | Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE );

    @ParameterizedTest
    @ValueSource(strings = {
            // CREATE
            "CREATE (a:Actor) RETURN a", "create (a:Actor) return a",
            // MERGE
            "MERGE (a:Actor {id : 10}) ON CREATE SET a.created = timestamp() ON MATCH SET a.accessTime = timestamp()",
            "merge (a:Actor {id : 10}) ON CREATE SET a.created = timestamp() ON MATCH SET a.accessTime = timestamp()",
            // SET
            "MATCH (a:Actor) SET a.age = 45", "match (a:Actor) set a.age = 45",
            // DELETE
            "MATCH (a:Actor) DELETE a", "match (a:Actor) delete a", "MATCH (a:Actor) DETACH DELETE a",
            "match (a:Actor) detach delete a",
            // REMOVE
            "MATCH (a:Actor) REMOVE a.age", "match (a:Actor) remove a.age",
            // DROP
            "DROP USER test",
            // CALL
            "call sp.doSomething()", "CALL sp.doSomething()", "MATCH (a:Actor) WITH a CALL sp.doSomething(a)",
            // CALL with misspelled OGM READ_ONLY hint
            "MATCH (a:Actor) WITH a CALL /*+ OGM_READ_ONLY */ sp.doSomething(a)",
            "MATCH (a:Actor) WITH a CALL /*+ OGM_READ_ONLY */ sp.doSomething(a)",
            "MATCH (a:Actor) WITH a CALL /*+ OGM READ ONLY */ sp.doSomething(a)",
            "MATCH (a:Actor) WITH a CALL /*+ OGM READ _ONLY */ sp.doSomething(a)",
            "MATCH (a:Actor) WITH a CALL /*+OGM READ_ONLY */ sp.doSomething(a)",
            "MATCH (a:Actor) WITH a CALL /*+ OGM READ_ONLY*/ sp.doSomething(a)",
            "MATCH (a:Actor) WITH a CALL /*+OGM READ_ONLY*/ sp.doSomething(a)", })
    void testWriteQueries( final String query ) {
        assertThat( this.pattern.matcher( query ).matches() ).isTrue();
    }

    @ParameterizedTest
    @ValueSource(strings = {
            // Simple match
            "MATCH (a:Actor) RETURN a", "match (a:Actor) return a",
            // CALL with OGM READ_ONLY-hint
            "call /*+ OGM READ_ONLY */ sp.doSomething()", "/*+ OGM READ_ONLY */ call sp.doSomething()",
            "call sp.doSomething() /*+ OGM READ_ONLY */", "MATCH (a:Actor) WITH a CALL /*+ OGM READ_ONLY */ sp.doSomething(a)" })
    void testReadOnlyQueries( final String query ) {
        assertThat( this.pattern.matcher( query ).matches() ).isFalse();
    }
}

[michael-simons]

Thank you people so much! I really like that approach (actually much better than mine) and I would incorporate it before our next release.
Next drink is on us (didn't you wanna do a meetup if I remember correctly?=

[torstenkuhnhenne]

Yes, we already hosted the JUG Paderborn Meetup last year when @meistermeier jumps in because of your travel problem. Maybe we can host another one when it is possible again, maybe we can talk to each other in October after my vacation.

Looking forward for the next release and thank you for your great work! Without Neo4j OGM and SDN our work would not be possible or much more complicated.

[michael-simons]

Thanks for your kind words. I have added your tests now + some multi line queries. I cannot get a regex ready to cater for those, but I moved the check for the hint into the method itself, very valid suggestions. Thanks!