dimas-b commented on code in PR #3385:
URL: https://github.com/apache/polaris/pull/3385#discussion_r2775688321
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -224,6 +225,29 @@ public PolarisMetaStoreManager polarisMetaStoreManager(
return metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
}
+ /**
+ * Produces a {@link MetricsPersistence} bean for the current request.
+ *
+ * <p>This method selects a MetricsPersistence implementation based on the
configured persistence
+ * type. If a backend-specific implementation is available (e.g., JDBC with
metrics schema), it
+ * will be used. Otherwise, falls back to the no-op implementation.
+ *
+ * @param config the persistence configuration
+ * @param metricsPersistenceImpls all available MetricsPersistence
implementations
+ * @return a MetricsPersistence implementation for the current realm
+ */
+ @Produces
+ @RequestScoped
+ public MetricsPersistence metricsPersistence(
+ PersistenceConfiguration config, @Any Instance<MetricsPersistence>
metricsPersistenceImpls) {
+ Instance<MetricsPersistence> selected =
+ metricsPersistenceImpls.select(Identifier.Literal.of(config.type()));
+ if (selected.isResolvable()) {
+ return selected.get();
+ }
+ return MetricsPersistence.NOOP;
Review Comment:
I'd rather error out if the configured instance is not resolvable. We do
that pretty much in all other cases.
`NOOP` could be selected with a default value in `application.properties`.
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -224,6 +225,29 @@ public PolarisMetaStoreManager polarisMetaStoreManager(
return metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
}
+ /**
+ * Produces a {@link MetricsPersistence} bean for the current request.
+ *
+ * <p>This method selects a MetricsPersistence implementation based on the
configured persistence
+ * type. If a backend-specific implementation is available (e.g., JDBC with
metrics schema), it
+ * will be used. Otherwise, falls back to the no-op implementation.
+ *
+ * @param config the persistence configuration
+ * @param metricsPersistenceImpls all available MetricsPersistence
implementations
+ * @return a MetricsPersistence implementation for the current realm
+ */
+ @Produces
+ @RequestScoped
+ public MetricsPersistence metricsPersistence(
+ PersistenceConfiguration config, @Any Instance<MetricsPersistence>
metricsPersistenceImpls) {
+ Instance<MetricsPersistence> selected =
+ metricsPersistenceImpls.select(Identifier.Literal.of(config.type()));
Review Comment:
Would it make sense to use a different config for `MetricsPersistence`? We
came some way to detach it from the JDBC metastore, I believe it would be nice
to also make it selectable independently.
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java:
##########
@@ -172,6 +172,13 @@ public synchronized Map<String, PrincipalSecretsResult>
bootstrapRealms(
datasourceOperations
.getDatabaseType()
.openInitScriptResource(effectiveSchemaVersion));
+
+ // Run the metrics schema script if requested
+ if (JdbcBootstrapUtils.shouldIncludeMetrics(bootstrapOptions)) {
Review Comment:
Just to sync up and not forget: I believe it would be nice to have a
separate bootstrap handler for the metrics schema, but that can be done as a
follow-up PR.
--
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]
