CODEWITHSUNDEEP
javaredisjedisapache-commons-pool
theGamblerRises
theGamblerRises

Reputation: 696

Is it a bad practise to pass jedis instance between methods?

I was referring to this SO question, and I did couple of additions in this benchmark test. The main problem is my apis are getting slow as load increases on server. I am using jedis pool configuration.

// get a new instance
    public synchronized Jedis getJedi() {
    try {
        return jedisPool.getResource();
    } catch (Exception e) {
        log.fatal("REDIS CONN ERR:", e);
        return null;
    }
    }

// intialize at start
    public void initialize() {
    if (jedisPool == null) {
        IniUtils cp = PropertyReader.getConnPoolIni();

        String host = cp.get(REDIS, REDIS_HOST);
        int port = Integer.parseInt(cp.get(REDIS, REDIS_PORT));
        String password = cp.get(REDIS, REDIS_PASSWORD);
        int timeout = Integer.parseInt(cp.get(REDIS, REDIS_TIMEOUT));

        JedisPoolConfig poolConfig = new JedisPoolConfig();
        poolConfig.setMaxTotal(Integer.parseInt(cp.get(REDIS, REDIS_MAX_TOTAL_CONNECTIONS)));
        poolConfig.setMaxIdle(Integer.parseInt(cp.get(REDIS, REDIS_MAX_IDLE)));
        poolConfig.setMinIdle(Integer.parseInt(cp.get(REDIS, REDIS_MIN_IDLE)));
        poolConfig.setMaxWaitMillis(Long.parseLong(cp.get(REDIS, REDIS_MAX_WAIT_TIME_MILLIS)));

        poolConfig.setTestOnBorrow(true);
        poolConfig.setTestOnReturn(true);
        poolConfig.setTestWhileIdle(true);

        if (password != null && !password.trim().isEmpty()) {
        jedisPool = new JedisPool(poolConfig, host, port, timeout, password);
        } else {
        jedisPool = new JedisPool(poolConfig, host, port, timeout);
        }

        test();

      }
    }

    @Override
    public void destroy() {
      if (jedisPool.isClosed() == false)
          jedisPool.destroy();
    }

    private void test() {
      try (Jedis test = getJedi()) {
        log.info("Testing Redis:" + test.ping());
      }
    }

And while using, I get Jedis instance in try-with-resources and works on it. I use very less pipelining and there are various calls to Redis, so each time a method call, a new jedis instance gets created.

As per SO question shared, my implementation will lead to very slow results. So, can I pass around Jedis instance to methods and work with pipeline as per business logic. Something like this -

  public void push5(int n) {
    try (Jedis jedi = redisFactory.getJedi()) {
        pushWithResource(n, jedi, 0);
    }
  }

  public void pushWithResourceAndPipe(int n, Jedis jedi, int k) {
    if (k >= n)
        return;
    Pipeline pipeline = jedi.pipelined();
    map.put("id", "" + i);
    map.put("name", "lyj" + i);
    pipeline.hmset("m" + i, map);
    ++i;
    pushWithResourceAndPipe(n, jedi, ++k);
    pipeline.sync();
  }

    public void pushWithResource(int n, Jedis jedi, int k) {
    if (k >= n)
        return;
    map.put("id", "" + i);
    map.put("name", "lyj" + i);
    jedi.hmset("m" + i, map);
    ++i;
    pushWithResource(n, jedi, ++k);
  }

Is there any way to improve on api calls? Could you recommend some projects which uses jedis on server side, so I will have a better understanding on how to use jedis effectively.

Redis / Jedis Configuration

Jedis version:2.8.1 Redis version:2.8.4 Java version:1.8

Upvotes: 1

Views: 751

Answers (1)

Jungtaek Lim
Jungtaek Lim

Reputation: 1708

  1. getJedi doesn't need to be 'synchronized', since JedisPool is thread-safe.
  2. If your logic can tolerate adding latency from setTestOnBorrow, you can disable the other things (setTestOnReturn, setTestWhileIdle). If your logic can tolerate retry, disabling setTestOnBorrow should help. (since it always do ping-pong check for every borrowing)
  3. Your pushWithResourceAndPipe has bug: you're creating pipeline instance for every recursive call, which can be 'slower' than pushWithResource. I think you intended to add all requests to one pipeline and sync, and then you need to pass pipeline instance and call sync from caller.

Upvotes: 1

Related Questions