rebase and upgrade neo4j 2025.03.0 #6336

wartraxx51 · 2025-04-18T14:52:55Z

This is a rebase of the work done by @fatih-acar #5241 from the develop branch for the upgrade of Neo4j from version 5.20.0 to 5.26.5.
There is a clear improvement in performance.

codspeed-hq · 2025-04-18T14:57:37Z

CodSpeed Performance Report

Merging #6336 will not alter performance

_{Comparing fac-04182025-upgrade-neo4j-5.26 (adf373f) with develop (12481ec)}

Summary

✅ 10 untouched benchmarks

Workaround until we update the sdk to use the latest testcontainers. Signed-off-by: Fatih Acar <[email protected]>

fatih-acar · 2025-04-25T08:23:29Z

25% improvement according to the testing framework on the TestSite test.

@wartraxx51 I squashed some commits in the branch after my own review.

Signed-off-by: Fatih Acar <[email protected]>

ajtmccarty

wow. this must have been very painful. very impressive.
it might be worth opening a 1.3 issue to make sure we do one final pass for any CALL subqueries added in stable and merged forward

backend/infrahub/core/validators/uniqueness/query.py

dgarros · 2025-04-28T06:56:25Z

backend/tests/unit/core/migrations/graph/test_020.py

            MATCH (a)-[ve:IS_VISIBLE]->(v)
-            WITH a, ve, v
+            WITH ve, v


I'm surprised by this one, make sense to remove the first line but later aren't we gonna loose the variable if we don't include it in WITH

The behavior changes with the new syntax.
According to: https://neo4j.com/docs/cypher-manual/current/subqueries/call-subquery/#variable-scope-rules

The scope clause’s variables can be globally referenced in the subquery. A subsequent WITH within the subquery cannot delist an imported variable. The deprecated importing WITH clause behaves differently because imported variables can only be referenced from the first line and can be delisted by subsequent clauses.

It would be indeed lost with the previous WITH method.

dgarros · 2025-04-28T06:57:58Z

.github/workflows/ci.yml

@@ -409,6 +409,7 @@ jobs:
    timeout-minutes: 45
    env:
      INFRAHUB_DB_TYPE: neo4j
+      NEO4J_DOCKER_IMAGE: "neo4j:2025.03.0-community"


is it required to define the version in GHA action or was it mainly for testing ?

That was required to make the CI pass.
The issue here is that we depend on the Infrahub SDK which imports an old infrahub-testcontainers version (with the previous neo4j version). That makes the docker integration tests fail with the new backend code...

Should we publish an alpha version of testcontainers, then an alpha version of the SDK, and then make develop point to the alpha SDK instead?

Ideally we should use the local version of infrahub-testcontainers, otherwise this issue will keep coming up when we have to change the docker compose

I removed the environment variable that was forcing the database version in the last commit and this part of ci

dgarros · 2025-04-28T06:58:26Z

.github/workflows/version-upgrade.yml

@@ -34,6 +34,7 @@ jobs:
        include:
          - name: From 1.1.0
            source_version: 1.1.0
+            neo4j_image: "neo4j:5.20.0-enterprise"


Same here, is it required to set the version here,

Since we rely on the development/docker-compose.yml stuff for the upgrade test job, we must start the previous Infrahub version with the previous neo4j version.

I don't know any other way to do that except specifying the neo4j docker image to use when starting the previous Infrahub version.

this one make sense

dgarros · 2025-04-28T07:03:20Z

backend/infrahub/core/query/node.py

            CREATE (a:Attribute { uuid: attr.uuid, name: attr.name, branch_support: attr.branch_support })
            CREATE (n)-[:HAS_ATTRIBUTE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(a)
            MERGE (av:AttributeValue { value: attr.content.value, is_default: attr.content.is_default })
-            WITH n, attr, av, a
+            WITH av, a


I'm surprised about this one too

dgarros · 2025-04-28T07:05:13Z

backend/infrahub/core/validators/uniqueness/query.py

@@ -137,12 +137,11 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None:  # noqa
        # ruff: noqa: E501
        query = """
        // get attributes for node and its relationships
-        CALL {
+        CALL () {


it feels like this shouldn't be empty, most likely we need to investigate what is generated as part of select_subqueries_str

This is legit: the whole query starts with a subquery, so there are no variables to import at this stage.

Signed-off-by: Fatih Acar <[email protected]>

fatih-acar · 2025-04-28T07:25:03Z

wow. this must have been very painful. very impressive. it might be worth opening a 1.3 issue to make sure we do one final pass for any CALL subqueries added in stable and merged forward

Good point, I created IFC-1482 internally.

fatih-acar · 2025-05-05T12:22:00Z

Thanks @wartraxx51 on the help here, I think we can merge this PR now.

wartraxx51 requested review from a team as code owners April 18, 2025 14:52

github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Apr 18, 2025

wartraxx51 force-pushed the fac-04182025-upgrade-neo4j-5.26 branch 5 times, most recently from 95613a5 to 950688a Compare April 22, 2025 10:31

fatih-acar changed the title ~~rebase and upgrade neo4j 5.26.5~~ rebase and upgrade neo4j 2025.03.0 Apr 25, 2025

wartraxx51 and others added 3 commits April 25, 2025 10:17

Upgrade to neo4j 5.26.5

716e9a7

Upgrade to neo4j 2025.03.0

2758daf

fix(ci): force neo4j 2025 usage

bcee032

Workaround until we update the sdk to use the latest testcontainers. Signed-off-by: Fatih Acar <[email protected]>

fatih-acar force-pushed the fac-04182025-upgrade-neo4j-5.26 branch from 950688a to bcee032 Compare April 25, 2025 08:21

github-actions bot added the group/ci Issue related to the CI pipeline label Apr 25, 2025

fix(ci): use previous neo4j version when testing upgrade

054fb15

Signed-off-by: Fatih Acar <[email protected]>

fatih-acar force-pushed the fac-04182025-upgrade-neo4j-5.26 branch from 503f19d to 054fb15 Compare April 25, 2025 15:28

ajtmccarty approved these changes Apr 25, 2025

View reviewed changes

backend/infrahub/core/validators/uniqueness/query.py Outdated Show resolved Hide resolved

dgarros reviewed Apr 28, 2025

View reviewed changes

fix(backend): remove query comments

2c49bd7

Signed-off-by: Fatih Acar <[email protected]>

fix cicd version neo4j

adf373f

wartraxx51 force-pushed the fac-04182025-upgrade-neo4j-5.26 branch from c000862 to adf373f Compare May 5, 2025 11:53

fatih-acar approved these changes May 5, 2025

View reviewed changes

wartraxx51 merged commit 1faaaac into develop May 7, 2025
35 checks passed

wartraxx51 deleted the fac-04182025-upgrade-neo4j-5.26 branch May 7, 2025 08:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rebase and upgrade neo4j 2025.03.0 #6336

rebase and upgrade neo4j 2025.03.0 #6336

wartraxx51 commented Apr 18, 2025

codspeed-hq bot commented Apr 18, 2025 •

edited

Loading

fatih-acar commented Apr 25, 2025

ajtmccarty left a comment

dgarros Apr 28, 2025

fatih-acar Apr 28, 2025

dgarros Apr 28, 2025

fatih-acar Apr 28, 2025

dgarros Apr 28, 2025

wartraxx51 May 5, 2025

dgarros Apr 28, 2025

fatih-acar Apr 28, 2025

dgarros Apr 28, 2025

dgarros Apr 28, 2025

dgarros Apr 28, 2025

fatih-acar Apr 28, 2025

fatih-acar commented Apr 28, 2025

fatih-acar commented May 5, 2025

rebase and upgrade neo4j 2025.03.0 #6336

rebase and upgrade neo4j 2025.03.0 #6336

Conversation

wartraxx51 commented Apr 18, 2025

codspeed-hq bot commented Apr 18, 2025 • edited Loading

Merging #6336 will not alter performance

Summary

fatih-acar commented Apr 25, 2025

ajtmccarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fatih-acar commented Apr 28, 2025

fatih-acar commented May 5, 2025

codspeed-hq bot commented Apr 18, 2025 •

edited

Loading