jedis
jedis copied to clipboard
Fix jedis & pipeline has the same client result in thread issue
Description
the pipeline has the same client with the jedis which get from jedis pool, if the jedis return to the pool, but the pipeline is still working, then the buf in RedisOutputStream maybe override, it will result in the wrong redis comand flush to redis server.
@lionhuo It is expected that the user would be careful to avoid these situations. There several aspects/issues involved in handling this in Jedis, at least in current structure.
@sazzad16 It is expected that the user would be careful to avoid these situations. yes, you are right
but it may be cause some confusion, for me, execute command, hset {key} {filed} {value}
got result:
filed: "*1\r\n$4\r\nPING\r\nbalabalabala"
of cause, it's a negative example
try(Jedis jedis = pool.getResouce()){
Pipeline pip = jedis.pipelined(); //if pip escapes from the block, may cause issue
}
so i think the pipeline's lifecycle is independent maybe better, generate a hidden jedis object just service pipeline, release after pipeline finish(close/clear/sync/syncAndReturnAll)
Jedis.java
@Override
public Pipeline pipelined() {
if (dataSource == null) {
return super.pipelined();
}
try{
Jedis jedis = this.dataSource.getResource();
Pipeline pipeline = super.pipelined(jedis.client);
pipeline.setJedis(jedis);
return pipeline;
}finally {
this.close();
}
}
BinaryJedis.java
public Pipeline pipelined(Client client) {
pipeline = new Pipeline();
pipeline.setClient(client);
return pipeline;
}
Pipeline.java
public void sync() {
try{
if (getPipelinedResponseLength() > 0) {
List<Object> unformatted = client.getMany(getPipelinedResponseLength());
for (Object o : unformatted) {
generateResponse(o);
}
}
}finally {
if(jedis != null){
jedis.close();
}
}
}
@yangbodong22011
the pipeline is no longer available, this is incompatible with the previous semantics
ok, noted
i think Pipeline
is the resource, it is the same with Jedis
, so could we get it from JedisPool
? and release after call close method
try(Pipeline pip = pool.getPipeline()){
......
}
JedisPool.java
public Pipeline getPipeline() {
Jedis jedis = this.getResource();
Pipeline pipeline = jedis.pipelined();
pipeline.setJedis(jedis);
return pipeline;
}
Pipeline.java
@Override
public void close() {
try{
clear();
}finally {
if(jedis != null){
jedis.close();
}
}
}
try(Pipeline pip = pool.getPipeline()){ ...... }
I (personally) think we don’t need it, the following code is just one more line.
try(Jedis jedis = pool.getPipeline()){
Pipeline pip = jedis.pipelined();
......
}
Let us wait for the opinions of others.
the following code is just one more line.
you are right
but i think get Pipeline
from the pool,It reinforces Pipeline
is a independent resource of Jedis
and user don't need careful to use Jedis
and Pipeline
if Pipeline
escapes from Jedis
block