dimas-b commented on code in PR #4397:
URL: https://github.com/apache/polaris/pull/4397#discussion_r3344948777


##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -136,8 +137,14 @@ public CallContext polarisCallContext(
       RealmContext realmContext,
       RealmConfigurationSource configurationSource,
       MetaStoreManagerFactory metaStoreManagerFactory) {
-    BasePersistence metaStoreSession = 
metaStoreManagerFactory.getOrCreateSession(realmContext);
-    return new PolarisCallContext(realmContext, metaStoreSession, 
configurationSource);
+    BasePersistence metaStore = 
metaStoreManagerFactory.getOrCreateSession(realmContext);
+    // When the backend implements both SPIs on the same instance (e.g. JDBC, 
in-memory), reuse the
+    // session instead of building a second persistence instance per request.
+    MetricsPersistence metricsPersistence =
+        (metaStore instanceof MetricsPersistence mp)
+            ? mp
+            : 
metaStoreManagerFactory.getOrCreateMetricsPersistence(realmContext);

Review Comment:
   This call is generally useless now. All functional implementations of 
`getOrCreateMetricsPersistence()` produce the objects that support both the 
`BasePersistence` and `MetricsPersistence` interfaces.
   
   If we want to keep the type check logic on line 144, I think we ought to 
remove `getOrCreateMetricsPersistence()` from `MetaStoreManagerFactory` for the 
sake of clarity.
   
   Alternatively, we can remove the type check on line 144 and rely on the 
factory to perform optimizations like returning the same object for both 
cases... when required.
   
   As for JDBC, I do not think such an optimization is required. The returned 
`JdbcBasePersistenceImpl` is a light-weight shell and creating two vs. one of 
them will hardly be noticeable in runtime.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to