bharos commented on code in PR #10787:
URL: https://github.com/apache/gravitino/pull/10787#discussion_r3102683993


##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java:
##########
@@ -220,7 +220,17 @@ public void close() throws Exception {
   }
 
   public Map<String, String> getCatalogConfigToClient() {
-    return catalogConfigToClients;
+    Map<String, String> configToClients = catalogConfigToClients;
+    if (configToClients != null) {
+      return configToClients;
+    }
+
+    synchronized (this) {

Review Comment:
   nit : The parent class uses a dedicated initializationLock object for all 
lazy init. The subclass uses synchronized(this). 
   Consider adding a private final Object lock = new Object() in the subclass 
for consistency ?



##########
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java:
##########
@@ -71,14 +71,14 @@ public class IcebergCatalogWrapper implements AutoCloseable 
{
 
   public static final Logger LOG = 
LoggerFactory.getLogger(IcebergCatalogWrapper.class);
 
-  @Getter protected Catalog catalog;
-  private SupportsNamespaces asNamespaceCatalog;
+  private final Object initializationLock = new Object();
+  protected volatile Catalog catalog;

Review Comment:
   Consider making it private to enforce access through getCatalog()?



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