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.

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

connectPool never returns second time #47

Open
GaikwadPratik opened this issue Feb 18, 2020 · 6 comments
Open

connectPool never returns second time #47

GaikwadPratik opened this issue Feb 18, 2020 · 6 comments
Labels
bug Something isn't working
Milestone

Comments

@GaikwadPratik
Copy link

@ronzeidman ,

This is more like a question than an issue. If I invoke await r.connectPool(dbOptions); multiple times(mistakenly), then only the first call returns, the second call or any subsequent calls to the above line never returns and kind of appear to be freezing.

Any idea, why?

@anli-xsigns
Copy link

anli-xsigns commented Mar 25, 2020

Yes, I spent the whole day finding the issue I had with my code (I had two connectPool too). The problem is in lines

if ((r as any).pool) {
((r as any).pool as MasterConnectionPool).removeAllListeners();
((r as any).pool as MasterConnectionPool).drain();
}
.
There an existing pool on the same connection will be drained if a new pool is created on the same connection. That can lead to silent errors in my opinion.

At least it is necessary to await the pool finishing the draining in line

((r as any).pool as MasterConnectionPool).drain();
before (r as any).pool is overwritten while draining in
(r as any).pool = cpool;

Maybe it is better to throw an error that a pool has to be closed manually before creating a new one instead of silently creating a new pool to avoid clumsy errors.

On the other hand MasterPool can handle an array of serverPools (see

this.serverPools = [];
), but that seems to be unused as it is recreated in
const cpool = new MasterConnectionPool({
...options,
servers
} as any);
(r as any).pool = cpool;

@atassis
Copy link
Collaborator

atassis commented Jul 28, 2020

@GaikwadPratik @anli-xsigns I might resolve this by splitting the logic into separate models, so r won't be a singleton which rules them all. r will only be needed to generate a query, while all connection handling, query running, etc will be handled by different pure functions

@anli-xsigns
Copy link

At the moment I'm not using this anymore. Your idea sounds good.

@atassis
Copy link
Collaborator

atassis commented Jul 29, 2020

@anli-xsigns you are not using rethinkdb or this exact library?

@anli-xsigns
Copy link

I'm not using rethinkdb anymore because work seems to be suspended again.

@atassis
Copy link
Collaborator

atassis commented Jul 29, 2020

Can't say thats a bad decision=)

@atassis atassis added the bug Something isn't working label Sep 12, 2020
@atassis atassis added this to the Version 3.0 milestone May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants