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

added lazy loading option for connection pool #37

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@coelhone
Copy link

coelhone commented Sep 2, 2013

Lazy loading option added.
Used passing :loading => :lazy to ConnectionPool's initializer (defaults to :eager).
When a connection is requested by the client, the pool's:

  1. creates one and returns it if it was empty (still have open slots - all connections were being used)
  2. returns one if it's available
  3. returns an ConnectionPool::ConnectionPoolFullException if all connection slots are being used
private

def create_connection
@existing_conns_count += 1

This comment has been minimized.

@mperham

mperham Sep 2, 2013

Owner

I'm unclear why the Pool needs to know the existing count. The connection creation and counting should all be tracked inside TimedStack IMO.

@@ -25,12 +25,22 @@ def push(obj)
end
alias_method :<<, :push

def pop(timeout=0.5)
deadline = Time.now + timeout
def pop(connections_count, timeout = 0.5)

This comment has been minimized.

@mperham

mperham Sep 2, 2013

Owner

Why would I need to pass a count to pop? That doesn't make a lot of sense. That's a sign you need to refactor your impl.

This comment has been minimized.

@coelhone

coelhone Sep 3, 2013

imho, the pool should be unaware of the counter (you're right) but not the
connection creation, i think...
the connection's associated stuff should be part of the pool and the
stack's ones should be part of timedstack.

the problem is, when creating some conenction (pool side) the counter should be incremented (stack side)

what do you think?

regards

This comment has been minimized.

@coelhone

coelhone Sep 3, 2013

moved counter to ConnectionPool::TimedStack and changed create_conenction method:

def create_connection
@available.increment_connection
@client_creation_block.call
end

@chandresh

This comment has been minimized.

Copy link

chandresh commented Sep 13, 2013

+1

@djanowski

This comment has been minimized.

Copy link
Collaborator

djanowski commented Sep 13, 2013

May I ask the use case for lazy loading?

@coelhone

This comment has been minimized.

Copy link

coelhone commented Sep 13, 2013

Hi, for example, you can have a global variable on your app creating a
client (in my case a cassandra one), that you want to use it with the app
and some tasks. Running other tasks that share the same environment, but
would not use the client, would create connections that wouldn't be used.
Connections will only be created on demand and if needed.

@djanowski

This comment has been minimized.

Copy link
Collaborator

djanowski commented Sep 13, 2013

Then why not simply define a global method (rather than a variable) which lazily creates the variable with the pool?

@coelhone

This comment has been minimized.

Copy link

coelhone commented Sep 13, 2013

That could be a solution, however, there would still be connections opened
to the DB that (pottencially) would never be used

2013/9/13 Damian Janowski notifications@github.com

Then why not simply define a global method (rather than a variable) which
lazily creates the variable with the pool?


Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-24397757
.

Bruno Coelho

@antonioleonardo

This comment has been minimized.

Copy link

antonioleonardo commented Sep 18, 2013

+1

@djanowski

This comment has been minimized.

Copy link
Collaborator

djanowski commented Sep 18, 2013

@coelhone If you never call the global method, then the pool won't be created. If this is such a big issue for you, I think you should create pools of objects which close connections after a certain amount of time.

I'm -1 on bringing this complexity to the gem for a quite contrived use case.

@coelhone

This comment has been minimized.

Copy link

coelhone commented Sep 18, 2013

I only gave you one example, the lazy load feature (IMHO) is best suited
for cases when you want to have the minimum required connections (at a
certain time, at a certain environment, at a certain task, etc...). This is
not a single use case and, has is referenced by the project, it is supposed
to be generic.
If i close connections after a certain amount of time, the problem of
having connections created without any use being given to them, still
persists.

Nevertheless, thanks for the reply

@coelhone coelhone closed this Sep 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment