Merge pull request #11255 from lassewesth/rcicleanup

cleaning up around request correlation id

Author: lassewesth

neo4j-integration-tools/src/main/java/org/neo4j/gds/integration/Neo4jPoweredRequestCorrelationId.java

@@ -33,8 +33,8 @@ private Neo4jPoweredRequestCorrelationId(String id) {
         this.id = id;
     }
 
-    public static Neo4jPoweredRequestCorrelationId create(long transactionId) {
-        return new Neo4jPoweredRequestCorrelationId(String.valueOf(transactionId));
+    public static Neo4jPoweredRequestCorrelationId create(long id) {
+        return new Neo4jPoweredRequestCorrelationId("gid-n4j-" + id);
     }
 
     @Override

procedures/facade/src/main/java/org/neo4j/gds/procedures/KernelTransactionAccessor.java

@@ -30,14 +30,8 @@
 public class KernelTransactionAccessor {
     public KernelTransaction getKernelTransaction(Context context) {
         try {
-            // this is not opening something new that we should close
-            // it only fetches the current transaction
-            // compare to what Neo4j does in: https://github.com/neo-technology/neo4j/blame/5.7.0/public/community/neo4j/src/main/java/org/neo4j/graphdb/facade/DatabaseManagementServiceFactory.java#L368-L369
-            // newer Neo4j versions offer a more direct method: https://github.com/neo-technology/neo4j/blob/e6a228f60efac4fd2584f5ec00de0207aad944ff/public/community/neo4j/src/main/java/org/neo4j/graphdb/facade/DatabaseManagementServiceFactory.java#L356
-            // we should make sure we do this in a compatible way
-            return context.internalTransaction().kernelTransaction();
+            return context.kernelTransaction();
         } catch (ProcedureException e) {
-            // Neo4j itself throws a different exception here; see https://github.com/neo-technology/neo4j/blob/340b40bb956d077b3127c8ba8eb33f2d288bc844/public/community/procedure/src/main/java/org/neo4j/procedure/impl/FieldSetter.java#L44-L49
             throw new RuntimeException(e);
         }
     }

procedures/facade/src/main/java/org/neo4j/gds/procedures/RequestCorrelationIdAccessor.java

@@ -21,24 +21,17 @@
 
 import org.neo4j.gds.core.RequestCorrelationId;
 import org.neo4j.gds.integration.Neo4jPoweredRequestCorrelationId;
-import org.neo4j.internal.kernel.api.exceptions.ProcedureException;
-import org.neo4j.kernel.api.procedure.Context;
+import org.neo4j.kernel.api.KernelTransaction;
 
 public class RequestCorrelationIdAccessor {
     /**
      * We use the transaction sequence number not because we particularly want to, but because Neo4j has limitations.
-     * The transaction id seems to not be available; and they do not offer a first class request id concept.
+     * The transaction id is not available, if you try to use it, you get this error:
+     * "Transaction id is not assigned yet. It will be assigned during transaction commit";
+     * and they do not offer a first class request id concept.
      * Here is to them improving that in future.
      */
-    public RequestCorrelationId getRequestCorrelationId(Context context) {
-        try {
-            @SuppressWarnings("resource")
-            var transactionSequenceNumber = context.kernelTransaction().getTransactionSequenceNumber();
-
-            return Neo4jPoweredRequestCorrelationId.create(transactionSequenceNumber);
-        } catch (ProcedureException e) {
-            // this must be a programmer error
-            throw new IllegalStateException("Unable to construct GDS", e);
-        }
+    public RequestCorrelationId getRequestCorrelationId(KernelTransaction kernelTransaction) {
+        return Neo4jPoweredRequestCorrelationId.create(kernelTransaction.getTransactionSequenceNumber());
     }
 }

procedures/integration/src/main/java/org/neo4j/gds/procedures/integration/GraphDataScienceProceduresProvider.java

@@ -37,7 +37,6 @@
 import org.neo4j.gds.core.utils.progress.TaskStoreService;
 import org.neo4j.gds.core.write.ExporterContext;
 import org.neo4j.gds.executor.MemoryEstimationContext;
-import org.neo4j.gds.integration.Neo4jPoweredRequestCorrelationId;
 import org.neo4j.gds.mem.MemoryTracker;
 import org.neo4j.gds.metrics.Metrics;
 import org.neo4j.gds.procedures.DatabaseIdAccessor;
@@ -48,6 +47,7 @@
 import org.neo4j.gds.procedures.LocalGraphDataScienceProcedures;
 import org.neo4j.gds.procedures.ProcedureCallContextReturnColumns;
 import org.neo4j.gds.procedures.ProcedureTransactionAccessor;
+import org.neo4j.gds.procedures.RequestCorrelationIdAccessor;
 import org.neo4j.gds.procedures.TaskRegistryFactoryService;
 import org.neo4j.gds.procedures.UserAccessor;
 import org.neo4j.gds.procedures.UserLogServices;
@@ -69,6 +69,7 @@ public class GraphDataScienceProceduresProvider implements ThrowingFunction<Cont
     private final DatabaseIdAccessor databaseIdAccessor = new DatabaseIdAccessor();
     private final KernelTransactionAccessor kernelTransactionAccessor = new KernelTransactionAccessor();
     private final ProcedureTransactionAccessor procedureTransactionAccessor = new ProcedureTransactionAccessor();
+    private final RequestCorrelationIdAccessor requestCorrelationIdAccessor = new RequestCorrelationIdAccessor();
     private final UserAccessor userAccessor;
 
     private final GdsLoggers loggers;
@@ -156,9 +157,9 @@ public GraphDataScienceProcedures apply(Context context) throws ProcedureExcepti
         var procedureCallContext = context.procedureCallContext();
         var procedureTransaction = procedureTransactionAccessor.getProcedureTransaction(context);
 
-        var correlationId = Neo4jPoweredRequestCorrelationId.create(kernelTransaction.getTransactionSequenceNumber());
         var databaseId = databaseIdAccessor.getDatabaseId(graphDatabaseService);
         var procedureReturnColumns = new ProcedureCallContextReturnColumns(procedureCallContext);
+        var requestCorrelationId = requestCorrelationIdAccessor.getRequestCorrelationId(kernelTransaction);
         var terminationMonitor = new TransactionTerminationMonitor(kernelTransaction);
         var terminationFlag = TerminationFlag.wrap(terminationMonitor);
         var user = userAccessor.getUser(context.securityContext());
@@ -181,7 +182,7 @@ public GraphDataScienceProcedures apply(Context context) throws ProcedureExcepti
         );
 
         var requestScopedDependencies = RequestScopedDependencies.builder()
-            .correlationId(correlationId)
+            .correlationId(requestCorrelationId)
             .databaseId(databaseId)
             .graphLoaderContext(graphLoaderContext)
             .taskRegistryFactory(taskRegistryFactory)