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


##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -140,13 +140,19 @@ public CallContext polarisCallContext(
       RealmConfigurationSource configurationSource,
       MetaStoreManagerFactory metaStoreManagerFactory) {
     BasePersistence metaStore = 
metaStoreManagerFactory.getOrCreateSession(realmContext);
+    return new PolarisCallContext(realmContext, metaStore, 
configurationSource);
+  }
+
+  @Produces
+  @RequestScoped
+  public MetricsPersistence metricsPersistence(
+      RealmContext realmContext, MetaStoreManagerFactory 
metaStoreManagerFactory) {
+    BasePersistence metaStore = 
metaStoreManagerFactory.getOrCreateSession(realmContext);

Review Comment:
   Why bother calling `getOrCreateSession()` now? This producer will be invoked 
by CDI only for use cases that need a `MetricsPersistence`, not 
`BasePersistence` 🤔 
   
   The old optimization of reusing the same object is no longer relevant.
   
   I think calling `getOrCreateMetricsPersistence()` (below) is sufficient.



##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/metrics/PolarisMetricsManager.java:
##########
@@ -73,6 +77,9 @@ default void writeScanMetrics(
    */
   default void writeCommitMetrics(
       @NonNull PolarisCallContext callCtx, @NonNull CommitMetricsRecord 
record) {
-    callCtx.getMetricsPersistence().writeCommitReport(record);
+    BasePersistence metaStore = callCtx.getMetaStore();
+    if (metaStore instanceof MetricsPersistence metricsPersistence) {

Review Comment:
   also not invoked, AFAIK.



##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/metrics/PolarisMetricsManager.java:
##########
@@ -56,7 +57,10 @@ public interface PolarisMetricsManager {
    */
   default void writeScanMetrics(
       @NonNull PolarisCallContext callCtx, @NonNull ScanMetricsRecord record) {
-    callCtx.getMetricsPersistence().writeScanReport(record);
+    BasePersistence metaStore = callCtx.getMetaStore();
+    if (metaStore instanceof MetricsPersistence metricsPersistence) {

Review Comment:
   This code is no longer invoked, I think... why not remove it?



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