The close method of MultiNodePipelineBase will repeatedly call the sync method.

[holidayQi]

Expected behavior

I use ClusterPipeline like this

        try (ClusterPipeline clusterPipeline = new ClusterPipeline(clusterConnectionProvider)) {
            ...
            clusterPipeline.sync();
            ...
        }

After upgrading from version 4.3.0 to 4.4.6, a performance degradation was noticed. Analyzing the cpu flame graph revealed that the degradation was due to the invocation of the close method. Upon inspecting the source code, it was found that the syncing would not be set to true after the sync method is executed, causing the close method to repeatedly call the sync method.

Jedis version: 4.4.6

Java version: 1.8

[sazzad16]

By repeatedly, you mean once, right?

MultiNodePipelineBase is updated to support intermediate/repeated sync() calls. Previously you could do sync() only once. So even though close() was calling sync() it was being ignored.

May I suggest to modify your code?

1. The simpler approach would be to remove sync() calls as close() would call it anyway.

2. Another approach, especially if you're using ClusterPipeline repeatedly is to reuse the same object and sync() method. For example, change

    for (int count = 0; count < max_count; count++) {
        try (ClusterPipeline clusterPipeline = new ClusterPipeline(clusterConnectionProvider)) {
            ...
            clusterPipeline.sync();
            ...
        }
    }

to

    ClusterPipeline clusterPipeline = new ClusterPipeline(clusterConnectionProvider);
    for (int count = 0; count < max_count; count++) {
        ...
        clusterPipeline.sync();
        ...
    }
    clusterPipeline.close();

[holidayQi]

Thank you for your reply. Removing the sync() can solve my issue, but it does seem a little strange as the sync logic is hidden within the close method.
Is it better to handle it this way that the sync() can still be called repeatedly, but once called, the close() will not call the sync().This feels more intuitive, and also provides compatibility with historical versions.

[sazzad16]

Actually the updated MultiNodePipelineBase behavior is the compatible one with the historical Pipeline class where you can make intermediate sync() calls.

Moreover, "close() will not call the sync()" actually breaks the historical Pipeline behavior.

[holidayQi]

Well, the incompatibility I am talking about is from a performance perspective. As mentioned above, just upgrading the version will cause a significant performance drop in high concurrency scenarios. If I want to maintain the previous performance, I need to modify the code, so it is said to be incompatible.
Or maybe I used it wrong before, the previous way of writing is not the recommended way to use it

        try (ClusterPipeline clusterPipeline = new ClusterPipeline(clusterConnectionProvider)) {
            ...
            clusterPipeline.sync();
            ...
        }

but I see that it is also written like this in ClusterPipeliningTest.