Pipeline#discard() replacement since Jedis version 4

[informatik01]

Hi.

I am in the process of upgrading Jedis dependency in one of our services from version 3.3.0 to the latest (as of now) Jedis version 4.4.3.

On the Jedis 4 Breaking Changes page it is mentioned:

The Redis transaction methods multi(), exec() and discard() have been removed from Pipeline.

In our code we have a helper method which uses Pipeline object and the logic is wrapped in a try-catch block. Here is the related catch block:

} catch (JedisException ex) {
    if (pipeline != null) {
        pipeline.discard();
    }
    throw ex;
}

As you can see, we used pipeline.discard() method call, but since version 4 - it is removed. What would be the closest equivalent (or is it needed at all) of the discard() call since Jedis version 4?

Would appreciate your help.

[sazzad16]

When not in Transaction (MULTI) mode, DISCARD doesn't make any sense. It's not clear what's happening before and after the codeblock. I guess you can close() or sync().

[informatik01]

Hi, @sazzad16.

Thank you for your comment.

Basically here the whole body of the method in question:

protected <TResult> TResult executeInPipeline(Function<Pipeline, TResult> func) {
    try (Jedis client = jedisPool.getResource()) {
        Pipeline pipeline = client.pipelined();
        TResult result;
        try {
            result = apply(func, pipeline);
            pipeline.sync();
            return result;
        } catch (JedisException ex) {
            if (pipeline != null) {
                pipeline.discard();
            }
            throw ex;
        }
    }
}

private <TParam, TResult> TResult apply(Function<TParam, TResult> func, TParam parameter) {
    return func.apply(parameter);
}

As we can see, in the core of this method there is a call to another method apply(func, pipeline) which is supposed to perform the "main work" (kind of exotic looking, with functional interfaces, but anyways). And IF during this work something goes wrong, then in that catch block there was an attempt to kind of "rollback" the commands in the pipleline, hence executing the discard() method which under the hood executes the DISCARD command.

As we can also see, the Jedis client object is created inside try-with-resources statement, so the related resource should be closed at the end. I was just worried that if previously the was a call to Pipeline#discard(), it was supposed to "bring some benefit" (at least it was there for some reason) and now it's not available.

So what do you think of that? Will it be totally OK to just omit the pipeline.discard() part and that's it?

[sazzad16]

@informatik01 Your code uses Transaction, not Pipeline. Keep what you have, including discard().

[informatik01]

@sazzad16
Ah, guess it's the lack of sleep today 🤣, but I have accidentally copied another method (we have a similar method that uses transactions too). And yes, for Transaction class the discard() method is left intact, so no problems with that case.

Fixed my previous message.
Sorry about that.

So what would you say now?

[sazzad16]

BTW, discard() right after exec() doesn't make sense either.

[sazzad16]

Remove discard(), that's all.

[informatik01]

1) Regarding pipelines.

So for pipelines the discard() call is not needed and it is totally fine to omit that "old" part. Would be nice if this would be briefly mentioned in the related migration document (Jedis 4 Breaking Changes) otherwise it was not clear why it was present in the first place and then just removed without any comment etc.

Thank you for confirming that with the new version it's OK to leave the code without Pipeline#discard().

---

2) Regarding your comment:

BTW, discard() right after exec() doesn't make sense either.

You mean in that accidentally posted example with transactions?
But the discard() is ONLY called in the catch block, so normally it won't be called after exec() there:

try {
    result = apply(func, transaction);
    transaction.exec();
    return result;
} catch (JedisException ex) {
    if (transaction != null) {
        transaction.discard();
    }
    throw ex;
}

Are you sure calling discard() is not needed in the catch block? Because the discard() method is still available for Transaction objects in the version 4 of Jedis.

I know that Redis does not support traditional rollbacks which are common for relational databases. But on the official Redis website there's an article - "How transactions work in Redis", and although they do mention that Redis does not support rollbacks, they also suggest using the DISCARD command as a replacement.

Here is the quote from that article (emphasis mine):

DISCARD can be used in order to abort a transaction. In this case, no commands are executed and the state of the connection is restored to normal.

I didn't write the logic for the examples I presented above, I am just in the process of updating the dependencies and your explanation would be really useful (we could remove that discard() call for the transactions related code too then, if it's not needed).

So could you please briefly elaborate why you think calling discard() in the catch block would be unnecessary for transactions too?

[sazzad16]

for pipelines the discard() call is not needed and it is totally fine to omit that "old" part. Would be nice if this would be briefly mentioned in the related migration document ...

Prior to Jedis 4, you could start a transaction within Pipeline class. This support removed since Jedis 4.
discard() is unnecessary if are not using transaction.
Your code was using discard() unnecessarily in Pipeline class (as it was not using transaction within that). It doesn't matter if it's Jedis 4 or older version.

[informatik01]

discard() is unnecessary if are not using transaction.

Yes, with pipelines everything is clear, thank you.

My only question was regarding your previous comment:

BTW, discard() right after exec() doesn't make sense either.

The exec() method was called on the transaction object in my previous "transaction related" example, that is way I wrote about it in item "2)" above.

That's the only question I have for now.

[sazzad16]

discard() is also unnecessary even if your are using transaction but not in transaction mode.

exec() declares the end of a transaction mode irrespective of it's success or failure. So after calling exec(), even if it fails, there is not an active transaction anymore. That's why I'm saying that discard() is unnecessary in your code.

Usually in same code flow you use either EXEC or DISCARD, not both. You call EXEC to try completing the transaction. Or call DISCARD to abort. You don't need DISCARD after EXEC because if the EXEC is successful the transaction is completed and if the EXEC is failed the transaction is aborted (already).

I guess, if it were (current) me, I would've written your code differently.

[informatik01]

As I mentioned I didn't write that code either, but it was present in one of our projects since 2017 ))

You don't need DISCARD after EXEC because if the EXEC is successful the transaction is completed and if the EXEC is failed the transaction is aborted (already).

This is really useful information. I now have no further questions.
I will update our code taking into consideration your comments.

---

Thank you very much for you clarifications, I really appreciate it!
Have a nice day.