singhpk234 commented on code in PR #4054:
URL: https://github.com/apache/polaris/pull/4054#discussion_r3011127810


##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -93,42 +114,48 @@ protected PolarisMetaStoreManager 
createNewMetaStoreManager() {
     return new AtomicOperationMetaStoreManager(clock, diagnostics);
   }
 
-  private void initializeForRealm(
-      DatasourceOperations datasourceOperations,
-      RealmContext realmContext,
-      RootCredentialsSet rootCredentialsSet) {
-    // Materialize realmId so that background tasks that don't have an active
-    // RealmContext (request-scoped bean) can still create a 
JdbcBasePersistenceImpl
-    String realmId = realmContext.getRealmIdentifier();
-    // determine schemaVersion once per realm
-    final int schemaVersion =
-        JdbcBasePersistenceImpl.loadSchemaVersion(
-            datasourceOperations,
-            
realmConfig.getConfig(BehaviorChangeConfiguration.SCHEMA_VERSION_FALL_BACK_ON_DNE));
-
-    sessionSupplierMap.put(
+  /** Loads and caches the schema version for the given realm (DB hit only on 
first call). */
+  private int getOrLoadSchemaVersion(DatasourceOperations 
datasourceOperations, String realmId) {
+    return schemaVersionCache.computeIfAbsent(
         realmId,
-        () ->
-            new JdbcBasePersistenceImpl(
-                diagnostics,
+        k ->
+            JdbcBasePersistenceImpl.loadSchemaVersion(
                 datasourceOperations,
-                secretsGenerator(realmId, rootCredentialsSet),
-                storageIntegrationProvider,
-                realmId,
-                schemaVersion));
+                realmConfig.getConfig(
+                    
BehaviorChangeConfiguration.SCHEMA_VERSION_FALL_BACK_ON_DNE)));
+  }
 
-    PolarisMetaStoreManager metaStoreManager = createNewMetaStoreManager();
-    metaStoreManagerMap.put(realmId, metaStoreManager);
+  /** Creates a new stateless {@link JdbcBasePersistenceImpl} for the given 
realm. */
+  private BasePersistence createSession(
+      DatasourceOperations datasourceOperations,
+      String realmId,
+      @Nullable RootCredentialsSet rootCredentialsSet) {
+    int schemaVersion = getOrLoadSchemaVersion(datasourceOperations, realmId);
+    return new JdbcBasePersistenceImpl(
+        diagnostics,
+        datasourceOperations,
+        secretsGenerator(realmId, rootCredentialsSet),
+        storageIntegrationProvider,
+        realmId,
+        schemaVersion);
   }
 
   public DatasourceOperations getDatasourceOperations() {
-    DatasourceOperations databaseOperations;
-    try {
-      databaseOperations = new DatasourceOperations(dataSource.get(), 
relationalJdbcConfiguration);
-    } catch (SQLException sqlException) {
-      throw new RuntimeException(sqlException);
+    DatasourceOperations ops = cachedDatasourceOperations;
+    if (ops == null) {
+      synchronized (this) {
+        ops = cachedDatasourceOperations;

Review Comment:
   I think the connection pool, inference of databaseType etc is what my 
motivation to cache it, please let me know if you feel strongly about it 
   
   agree on making the random number generator static if not already !



-- 
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