JRedisGraph icon indicating copy to clipboard operation
JRedisGraph copied to clipboard

How to close the redisgraph connections

Open BhanuPrakash531 opened this issue 5 years ago • 20 comments

Hi Team,

I'm looking for a template how to close the redisgraph connection obtained from the JedisPool to handle the resource leakage. How can we achieve this on JRedisGraph dependency?

Below is an excerpt of the code that gets the RedisGraph API which is used by other classes to execute queries against the database.

@Configuration
public class Config {
   @Bean
   public RedisGraph redisGraph() {
      JedisPool pool = new JedisPool(new JedisPoolConfig(), host, port, timeout, password);
      return new RedisGraph(pool);
   }
}

We are then calling the created bean directly to send query to database, this is resulting on resource leakage

@Service
public class Util {
   @Autowired
   private RedisGraph redisGraph; 
 public ResultSet redisGraphApi(String query) {
      ResultSet resultSet = redisGraph.query(graphId, query);
      return resultSet;
   }
}

BhanuPrakash531 avatar Feb 12 '20 09:02 BhanuPrakash531

Thanks, @BhanuPrakash531 Can you please send the leak log? I think I have a possible solution but I want to verify.

DvirDukhan avatar Feb 12 '20 10:02 DvirDukhan

What kind of leak log are you referring here?

BhanuPrakash531 avatar Feb 13 '20 10:02 BhanuPrakash531

@BhanuPrakash531 where did you see the resource leakage?

DvirDukhan avatar Feb 17 '20 08:02 DvirDukhan

We get too many open connections errors on the logs and database is the only connection we have in our application

BhanuPrakash531 avatar Feb 17 '20 12:02 BhanuPrakash531

@BhanuPrakash531 can you provide a code sample so I can reproduce it?

DvirDukhan avatar Feb 18 '20 15:02 DvirDukhan

Configuration:

`@Configuration
public class Config {
   @Bean
   public RedisGraph redisGraph() {
      JedisPool pool = new JedisPool(new JedisPoolConfig(), host, port, timeout, password);
      return new RedisGraph(pool);
   }
}`

Service

`@Service
public class Util {
   @Autowired
   private RedisGraph redisGraph; 
 public ResultSet redisGraphApi(String query) {
      ResultSet resultSet = redisGraph.query(graphId, query);
      return resultSet;
   }
}`

Service caller

`@Service
public class Controller {
   @Autowired
   private Util  util ; 

   public Employee getEmployee() {
      ResultSet resultSet = util.redisGraphApi(query);
      if (resultSet.hasNext()) {
        return buildEmployee(resultSet.next());
      }
   }
}`

Health Check:

`private boolean isDBConnected() {
      boolean isConnected = false;
      try (JedisPool pool = new JedisPool(new JedisPoolConfig(), host, port, timeout, password)) {
         if (pool.getResource().ping().equalsIgnoreCase("PONG"))
            isConnected = true;
      }
      return isConnected;
}`

BhanuPrakash531 avatar Feb 19 '20 07:02 BhanuPrakash531

hi @BhanuPrakash531 you try-with-resource a new Jedis pool, and check if you can get a response from the server. The close method of the pool is closing the connections anyhow, once the `try-with-resource is done. can you please attach an example of a code that calls a redisgraph query and causes resource leakage?

DvirDukhan avatar Feb 19 '20 09:02 DvirDukhan

Added the Service caller above which calls the redisgraph.query. The method isConnected() is for health check of the database. this method is called every 100ms. Awful lot of times. After some time, cpu is reaching 100% and failing the system. Thread dumps says 195 threads in sun.misc.Unsafe.park(Native Method)

http-nio-8080-exec-200 priority:5 - threadId:0x00007fc49c0ef800 - nativeId:0xc83f - nativeId (decimal):51263 - state:WAITING stackTrace: java.lang.Thread.State: WAITING (parking) at sun.misc.Unsafe.park(Native Method) parking to wait for <0x00000000c026b6a0> (a java.util.concurrent.locks.ReentrantReadWriteLock$FairSync) at java.util.concurrent.locks.LockSupport.park(LockSupport.java:175) at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:836) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(AbstractQueuedSynchronizer.java:870) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1199) at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.lock(ReentrantReadWriteLock.java:943) at com.sun.jmx.mbeanserver.Repository.remove(Repository.java:623) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.unregisterFromRepository(DefaultMBeanServerInterceptor.java:1940) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.exclusiveUnregisterMBean(DefaultMBeanServerInterceptor.java:448) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.unregisterMBean(DefaultMBeanServerInterceptor.java:415) at com.sun.jmx.mbeanserver.JmxMBeanServer.unregisterMBean(JmxMBeanServer.java:546) at org.apache.commons.pool2.impl.BaseGenericObjectPool.jmxUnregister(BaseGenericObjectPool.java:852) at org.apache.commons.pool2.impl.GenericObjectPool.close(GenericObjectPool.java:699) locked <0x00000000f6928e30> (a java.lang.Object) at redis.clients.jedis.util.Pool.closeInternalPool(Pool.java:100) at redis.clients.jedis.util.Pool.destroy(Pool.java:87) at redis.clients.jedis.util.Pool.close(Pool.java:29) at com.bhanu.StatusCheck.isDBConnected(SvcLBStatusHandler.java:69)

BhanuPrakash531 avatar Feb 19 '20 14:02 BhanuPrakash531

@BhanuPrakash531 it seems like the fact you're opening so many connection pools so fast is overloading the JMX unregister. The best practice will be to use the same thread pool and just recycle the connections and not opening new pool every 100ms.

private boolean isDBConnected() {
      boolean isConnected = false;
      try (Jedis jedis = this.pool.getResource()) {
         if (jedis.ping().equalsIgnoreCase("PONG"))
            isConnected = true;
      }
      return isConnected;
}

gkorland avatar Feb 19 '20 16:02 gkorland

@BhanuPrakash531 I'm waiting for your input on @gkorland answer.

DvirDukhan avatar Feb 19 '20 16:02 DvirDukhan

Agreed @gkorland, for health checks to database servers, whats the recommended approach to handle this scenario from java services by using RedisGraph APIs?

BhanuPrakash531 avatar Feb 20 '20 09:02 BhanuPrakash531

What is the reason for these checks? are you running a general Redis monitor?

gkorland avatar Feb 20 '20 10:02 gkorland

No. We have developed an application on top of redis graph, other services which uses our service does a frequent healt checks on our application, we need to check the health of redis database for each of the request we get. Hence we need that operation

BhanuPrakash531 avatar Feb 20 '20 10:02 BhanuPrakash531

@BhanuPrakash531 you need a health check for the RedisGraph API? Do you want me to expose such an API?

DvirDukhan avatar Feb 20 '20 11:02 DvirDukhan

@BhanuPrakash531 In second thought, you can get a dedicated Jedis resource from the RedisGraphAPI by

boolean isConnected = false;
try(RedisGraphContext context = redisGraphApi.getContext()) {
    try (Jedis jedis =  context.getConnectionContext()) {
         if (jedis.ping().equalsIgnoreCase("PONG"))
            isConnected = true;
      }
}
return isConnected;

so you'll have full Jedis API as well as GraphAPI with this connection.

DvirDukhan avatar Feb 20 '20 13:02 DvirDukhan

Okay. Thanks for the above block of code, that helps. One last question, where does the connection opened for redisGraphApi closed here? Or that might not be closed? Or is that handled internally by JRedisGraph API?

BhanuPrakash531 avatar Feb 20 '20 13:02 BhanuPrakash531

you mean to

 JedisPool pool = new JedisPool(new JedisPoolConfig(), host, port, timeout, password);
 return new RedisGraph(pool);

? RedisGraphAPI is closable, once you close it, it will close the underlying pool.

DvirDukhan avatar Feb 20 '20 14:02 DvirDukhan

So that will be closed when the server is stopped? Or is there a best practice to close it?

BhanuPrakash531 avatar Feb 20 '20 14:02 BhanuPrakash531

@BhanuPrakash531 This is from Spring-RedisGraph repo, use the @Bean(destroyMethod = "close").

@Configuration
@ConfigurationProperties(prefix = "spring.redisgraph")
@Data
@ComponentScan
public class RedisGraphConfiguration {

    @Autowired
    @Getter(AccessLevel.NONE)
    @Setter(AccessLevel.NONE)
    private RedisProperties props;

    private String host;
    private Integer port;

    // Login to the database and set the password of redis graph with the below command
    // redis-cli$: CONFIG SET requirepass "Redis@password123"
    private String password;

    @Bean(destroyMethod = "close")
    public RedisGraph redisGraphConnection() {
        if (null == password) {
            return new com.redislabs.redisgraph.impl.api.RedisGraph();
        }
        else {
            JedisPool pool = new JedisPool(new JedisPoolConfig(), host, port, Protocol.DEFAULT_TIMEOUT, password);
            return new com.redislabs.redisgraph.impl.api.RedisGraph(pool);
        }
    }

    private String host() {
        if (host == null) {
            return props.getHost();
        }
        return host;
    }

    private int port() {
        if (port == null) {
            return props.getPort();
        }
        return port;
    }

    private String password() {
        if (password == null) {
            return props.getPassword();
        }
        return password;
    }
}

DvirDukhan avatar Feb 20 '20 20:02 DvirDukhan

Alright, thanks for the response. This helps. Appreciated.

BhanuPrakash531 avatar Feb 24 '20 10:02 BhanuPrakash531