Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stackoverflow in MultiResponseBuilder #856

Closed
DivineTraube opened this issue Jan 15, 2015 · 5 comments
Closed

Stackoverflow in MultiResponseBuilder #856

DivineTraube opened this issue Jan 15, 2015 · 5 comments

Comments

@DivineTraube
Copy link

Hi,

if one performs many operations (a few thousand) in a multi over a pipeline, the recursive nature of the response processing in Jedis will always cause a Stackoverflow.

Code to reproduce this in Jedis 2.6.2 :

Jedis jedis = new Jedis("localhost");
Pipeline p = jedis.pipelined();
p.multi();
for (int i = 0; i < 10000; i++) {
            p.setbit("test", 1, true); //or any other call
 }
Response<List<Object>> exec = p.exec();
p.sync();
exec.get();  //Booom

Stacktrace:

java.lang.StackOverflowError
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:35)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:31)
    at redis.clients.jedis.Pipeline$MultiResponseBuilder.build(Pipeline.java:12)
    at redis.clients.jedis.Response.build(Response.java:50)
    at redis.clients.jedis.Response.get(Response.java:28)
@DivineTraube DivineTraube mentioned this issue Jan 15, 2015
Closed
@marcosnils
Copy link
Contributor

@DivineTraube as you may already know this is a common issue when dealing with recursive calls in java as the thread stack size has a small default value (1024k).

The example below makes 50k recursive calls without allocating any variables inside the stack and also throws a a StackOverFlow error.

public class MainTest {

    public static int counter = 0;

    public static void main(String[] args) {
    sum();
    }

    private static void sum() {
    if (counter++ <= 50000) {
        sum();
    }
    }
}

If you're planning to make a lot of calls in the same pipeline I'd recommend to increase the thread stack size using the -Xss jvm parameter to suit your needs. Per your example a value of -Xss2048k should be enough to make 10k pipeline calls.

@HeartSaVioR
Copy link
Contributor

@DivineTraube
Thanks for reporting. I'll investigate on it.
@marcosnils
Maybe we have to find a nicer way to take care of dependency without cross reference, which makes recursion. Please share any ideas or approaches anytime when you'll have.

@HeartSaVioR
Copy link
Contributor

Maybe I got fixed it by #860.

I can't remove cross reference cause we cannot know what response.get() is called first.
(It can be "exec"'s response, or "request in multi"'s response.)
Instead, I changed Response.build() to prevent recursion by checking build is in progress.

@DivineTraube
Copy link
Author

Great, merging this would be highly appreciated

marcosnils added a commit that referenced this issue Feb 4, 2015
@marcosnils
Merge pull request #860 from HeartSaVioR/not-allowing-build-recursion…
dc9ba6d 
…-on-pipeline-with-multi

Prevent recursion from Response.build() with dependency (Fixes #856)
HeartSaVioR added a commit that referenced this issue Feb 4, 2015
@HeartSaVioR @marcosnils
Prevent recursion from Response.build() with dependency
8216735 
* Fixes #856
* Before patch it, users could experience StackOverflowError
** when there's multi in Pipeline, which has massive requests

Conflicts:
	src/test/java/redis/clients/jedis/tests/PipeliningTest.java
marcosnils added a commit that referenced this issue Feb 4, 2015
@marcosnils
Merge pull request #860 from HeartSaVioR/not-allowing-build-recursion…
5a299ba 
…-on-pipeline-with-multi

Prevent recursion from Response.build() with dependency (Fixes #856)
Conflicts:
	src/test/java/redis/clients/jedis/tests/PipeliningTest.java
@marcosnils
Copy link
Contributor

Merged to master, 2.7 and 2.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@HeartSaVioR @DivineTraube @marcosnils