nastra commented on code in PR #6410:
URL: https://github.com/apache/iceberg/pull/6410#discussion_r1054443705
##########
core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java:
##########
@@ -51,7 +53,12 @@ public Table loadTable(TableIdentifier identifier) {
}
} else {
- result = new BaseTable(ops, fullTableName(name(), identifier));
+ MetricsReporter metricsReporter =
Review Comment:
I don't think we would want to init a reporter on every single `loadTable()`
call. We should rather have a lazy `reporter` field that is initialized once.
Something like this will work:
```
diff --git a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
index 16a5164f6..8d5258620 100644
--- a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
@@ -25,6 +25,8 @@ import
org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.LoggingMetricsReporter;
+import org.apache.iceberg.metrics.MetricsReporter;
import org.apache.iceberg.relocated.com.google.common.base.MoreObjects;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
@@ -35,6 +37,19 @@ import org.slf4j.LoggerFactory;
public abstract class BaseMetastoreCatalog implements Catalog {
private static final Logger LOG =
LoggerFactory.getLogger(BaseMetastoreCatalog.class);
+ private MetricsReporter reporter;
+
+ private MetricsReporter reporter() {
+ if (null == reporter) {
+ this.reporter =
+ properties().containsKey(CatalogProperties.METRICS_REPORTER_IMPL)
+ ? CatalogUtil.loadMetricsReporter(
+ properties().get(CatalogProperties.METRICS_REPORTER_IMPL))
+ : LoggingMetricsReporter.instance();
+ }
+
+ return reporter;
+ }
@Override
public Table loadTable(TableIdentifier identifier) {
@@ -51,7 +66,7 @@ public abstract class BaseMetastoreCatalog implements
Catalog {
}
} else {
- result = new BaseTable(ops, fullTableName(name(), identifier));
+ result = new BaseTable(ops, fullTableName(name(), identifier),
reporter());
}
} else if (isValidMetadataIdentifier(identifier)) {
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
