[GitHub] [pulsar] merlimat commented on a change in pull request #10441: [pulsar-io] Using ObjectMapper instead of Gson to parse Source/SInk configuration

[GitBox <gi...@apache.org>]

merlimat commented on a change in pull request #10441:
URL: https://github.com/apache/pulsar/pull/10441#discussion_r623225368



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
##########
@@ -197,6 +197,7 @@ private PulsarClientImpl(ClientConfigurationData conf, EventLoopGroup eventLoopG
         } catch (Throwable t) {
             shutdown();
             shutdownEventLoopGroup(eventLoopGroup);
+            closeConnectionPool(cnxPool);

Review comment:
       The changes on `PulsarClientImpl` are not related with gson -> jackson and should go on their own separate commit.

##########
File path: pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java
##########
@@ -130,6 +130,8 @@
     // a read write lock for stats operations
     private ReadWriteLock statsLock = new ReentrantReadWriteLock();
 
+    private static final ObjectMapper objectMapper = new ObjectMapper();

Review comment:
       Object mapper is not always thread safe (eg: when caching the reflection scan). We already have a thread-local utility to get an object mapper with the right configuration (`ObjectMapperFactory.getThreadLocal()`). 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org