When high qps, async operation code stack may too deep

[semistone]

No description provided.

[mrniko]

Thanks for suggested changes. Can you share the example of stack you are trying to avoid?

[semistone]

stack.gz

it's related to issue
#5971

from the debug log implement by me in my debug branch, I could confirm that thread disappear.
so the semaphore never release in that test.
I also shared how to reproduce it in that issue.

[semistone]

it's simple POC about reduce call stack in AsyncSemaphore

    private void tryRun() {
        while (true) {
            if (counter.decrementAndGet() >= 0) {
                CompletableFuture<Lock> future = listeners.poll();
                if (future == null) {
                    counter.incrementAndGet();
                    return;
                }

                if (!future.isDone()) {
                    // release only once by lock object
                    AtomicBoolean once = new AtomicBoolean();
                    AtomicBoolean tryNext = new AtomicBoolean();
                    long currentThreadId = Thread.currentThread().getId();
                    if (future.complete(() -> { <==== return release runnable 
                        if (once.compareAndSet(false, true)) {
                            // if in the same thread and tryNext is false means already release in the same thread.
                            release(Thread.currentThread().getId() != currentThreadId || tryNext.compareAndSet(false, true));
                        }
                    })) {
                        if (tryNext.compareAndSet(false, true)) { 
                            continue; // continue to tryRun  if not release yet.
                        } else {
                            return;
                        }
                    }
                }
            }

            if (counter.incrementAndGet() <= 0) {
                return;
            }
        }
    

I didn't test and optimism it yet.

[mrniko]

you need to define serviceManager.getGroup() as an executor.

[mrniko]

serviceManager.getGroup().submit(() -> {
                releaseConnection();
});

in connectTo method now can be replaced with releaseConnection() method call only;

[semistone]

I change to always switch thread during release as your suggestion.
I checked when that error happen, it throw StackOverflowError and it's not Exception but throwable.
and the stack size is also depend on how user's implementation, it could happen after very few recursive loop.
so switch thread in every release is is better.

how to you think ?

[semistone]

or switch in ?

RedisExecutor.sendCommand
            if (connectionManager.getServiceManager().getConfig().getMasterConnectionPoolSize() < 10
                    && !command.isBlockingCommand()) {
                release(connection); <=== here
            }

or

    public void release() {
        counter.incrementAndGet();
        tryRun(); <=== here
    }

[mrniko]

Thanks for your assistance. I have fixed it in 3c1c90f