141 7 true 2009-01-13T22:36:38+00:00 37814 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 1058 8994 resolved activerecord connection patch pool 2009-05-01T14:58:51+01:00 17393 Repository coderrr Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x More useful version of with_connection. If you wrap all your code which makes AR queries with this method you don't have to worry about connection cleanup. More useful version of with_connection. If you wrap all your code which makes AR queries with this method you don't have to worry about connection cleanup. <div><p>More useful version of with_connection. If you wrap all your code which makes AR queries with this method you don't have to worry about connection cleanup.</p></div> 0 More useful version of with_connection. If you wrap all your code which makes AR queries with this method you don't have to worry about connection cleanup. It simply release the connection when the block is done. It also protects against releasing the connection inside of nested calls, it will only release when the topmost call completes (see test for example of this) The following code will leave 1 connection checked out of the connection pool Thread.new do ActiveRecord::Base.connection_pool.with_connection do |this_conn_isnt_actually_used| User.all end end.join But with cleanup_connection the connection will be checked in after the block completes. The connection pool will be in its original state when this code finishes. Thread.new do ActiveRecord::Base.connection_pool.cleanup_connection do User.all end end.join <div><p>More useful version of with_connection. If you wrap all your code which makes AR queries with this method you don't have to worry about connection cleanup. It simply release the connection when the block is done. It also protects against releasing the connection inside of nested calls, it will only release when the topmost call completes (see test for example of this)</p> <p>The following code will leave 1 connection checked out of the connection pool</p> <pre> <code>Thread.new do ActiveRecord::Base.connection_pool.with_connection do |this_conn_isnt_actually_used| User.all end end.join </code> </pre> <p>But with cleanup_connection the connection will be checked in after the block completes. The connection pool will be in its original state when this code finishes.</p> <pre> <code>Thread.new do ActiveRecord::Base.connection_pool.cleanup_connection do User.all end end.join </code> </pre></div> false 2009-01-13T22:36:38+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new patch 2009-01-16T00:23:38+00:00 37814 coderrr coderrr http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 1 false 2009-01-13T23:03:02+00:00 37814 --- :tag: patch :assigned_user: :title: "added using_connection method; like with_connection but sets it as default connection while inside the block " 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-01-13T23:03:03+00:00 1366 Pratik coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 1 example of why nesting protection is very helpful @@@ ruby def x using_connection do User.all.each {|u| y(u) } end end def y(u) using_connection do u.something = rand u.save! end end def z y(@user) end @@@ Basically it allows you to only wrap code which is immediately doing an AR update without having to worry about nesting issues. <div><p>example of why nesting protection is very helpful</p> <pre><code class="ruby"> def x using_connection do User.all.each {|u| y(u) } end end def y(u) using_connection do u.something = rand u.save! end end def z y(@user) end </code></pre> <p>Basically it allows you to only wrap code which is immediately doing an AR update without having to worry about nesting issues.</p></div> false 2009-01-14T06:29:43+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-01-14T06:29:46+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 1 Realized all I really care about is cleaning up the connection at the end of the block, so here's a simpler version. It doesn't explicitly check out the connection. That will be done by AR on the first call to AR::B.connection. Test for nesting safety added. <div><p>Realized all I really care about is cleaning up the connection at the end of the block, so here's a simpler version. It doesn't explicitly check out the connection. That will be done by AR on the first call to AR::B.connection.</p> <p>Test for nesting safety added.</p></div> false 2009-01-14T19:58:42+00:00 37814 --- :title: Adding using_connection method 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-01-14T19:58:47+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 2 added more real-world test case for nesting safety <div><p>added more real-world test case for nesting safety</p></div> false 2009-01-14T22:03:34+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-01-14T22:03:37+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 3 renamed thread local variable to cleanup_connection <div><p>renamed thread local variable to cleanup_connection</p></div> false 2009-01-14T22:05:42+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-01-14T22:05:48+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 4 +1 <div><p>+1</p></div> false 2009-01-17T22:59:23+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-01-17T22:59:24+00:00 21029 hosiawak coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 4 +1 This patch lets me use ActiveRecord in Ramaze. A most timely patch. <div><p>+1</p> <p>This patch lets me use ActiveRecord in Ramaze. A most timely patch.</p></div> false 2009-02-04T05:35:53+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-04T05:35:57+00:00 46153 Michael Lang coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 4 Moved cleanup_connection method to ConnectionHandler. Using clear_active_connections! instead of release_connection so we release connections from all connection pools in use by the current thread. <div><p>Moved cleanup_connection method to ConnectionHandler. Using clear_active_connections! instead of release_connection so we release connections from all connection pools in use by the current thread.</p></div> false 2009-02-17T21:31:53+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-17T21:31:57+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 5 As requested by lifo Real World Usecase: You have 100 threads which stay alive and running throughout the entire life of your process. These threads are intermittently doing AR calls. Now assume you can't or don't want to increase your connection pool size to 100. Or assume instead of 100 its some unknown number of threads which might be higher than you can set your connection pool size to. Without this patch each thread will hold onto a single connection even though it's not making use of it most of the time. With this patch you can wrap all your AR access in a cleanup_connection block and the thread will release the connection whenever its finished with it. This will allow you to get away with a small connection pool even with lots of threads using AR. <div><p>As requested by lifo</p> <p>Real World Usecase:</p> <p>You have 100 threads which stay alive and running throughout the entire life of your process. These threads are intermittently doing AR calls. Now assume you can't or don't want to increase your connection pool size to 100. Or assume instead of 100 its some unknown number of threads which might be higher than you can set your connection pool size to.</p> <p>Without this patch each thread will hold onto a single connection even though it's not making use of it most of the time. With this patch you can wrap all your AR access in a cleanup_connection block and the thread will release the connection whenever its finished with it. This will allow you to get away with a small connection pool even with lots of threads using AR.</p></div> false 2009-02-17T22:25:52+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-17T22:25:58+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 5 I can appreciate the desire for this patch, but what about just piggy-backing off the existing connection method instead? Consider this patch: http://gist.github.com/66038 It's just a proof-of-concept, may need some cleaning up and adding tests/merging w/ coderrr's patch. What do you think? <div><p>I can appreciate the desire for this patch, but what about just piggy-backing off the existing connection method instead? Consider this patch:</p> <p><a href="http://gist.github.com/66038">http://gist.github.com/66038</a></p> <p>It's just a proof-of-concept, may need some cleaning up and adding tests/merging w/ coderrr's patch.</p> <p>What do you think?</p></div> false 2009-02-17T22:48:38+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-17T22:48:41+00:00 17057 Nick Sieger coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 5 Hey nick The only issue with that is that if you have multiple connections (in different AR models) you'd need to do a connection call for each of them. If you forgot to do that these connections would remain open in the thread. User.connection { Project.connection { ... } } Not that big deal. Other than that I don't really have any objection to piggybacking. <div><p>Hey nick</p> <p>The only issue with that is that if you have multiple connections (in different AR models) you'd need to do a connection call for each of them. If you forgot to do that these connections would remain open in the thread.</p> <p>User.connection { Project.connection { ... } }</p> <p>Not that big deal. Other than that I don't really have any objection to piggybacking.</p></div> false 2009-02-17T23:01:59+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-17T23:02:01+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 5 If you're satisfied with that pattern, we could push the logic up to the AR::Base class method to do the cleanup and it could do so with clear_active_connections! like you have in your patch. Any other thoughts about the overloading of AR::Base.connection? <div><p>If you're satisfied with that pattern, we could push the logic up to the AR::Base class method to do the cleanup and it could do so with clear_active_connections! like you have in your patch.</p> <p>Any other thoughts about the overloading of AR::Base.connection?</p></div> false 2009-02-18T01:48:12+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-18T01:48:16+00:00 17057 Nick Sieger coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 5 Here's a patch w/ coderrr's tests and my approach with passing the block down through AR::Base.connection. <div><p>Here's a patch w/ coderrr's tests and my approach with passing the block down through AR::Base.connection.</p></div> false 2009-02-18T20:28:48+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-18T20:28:50+00:00 17057 Nick Sieger coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 Don't we already have a with_connection method on the connection pool? Perhaps we just make that available from AR::Base more simply? <div><p>Don't we already have a with_connection method on the connection pool?</p> <p>Perhaps we just make that available from AR::Base more simply?</p></div> false 2009-02-18T20:52:19+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-18T20:52:23+00:00 141 Michael Koziarski coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 with_connection isn't good enough. It only releases the connection which it checks out. And the connection it checks out will be different from AR::B#connection <div><p>with_connection isn't good enough. It only releases the connection which it checks out. And the connection it checks out will be different from AR::B#connection</p></div> false 2009-02-18T20:55:11+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-18T20:55:12+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 +1 <div><p>+1</p></div> false 2009-02-19T14:36:40+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-19T14:36:46+00:00 22242 Dan Pickett coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 Koz: with_connection doesn't associate the connection with the thread, so that regular code like Model.find would still pull another connection. That's yet another option to make with_connection stash the connection away in the thread if it isn't there. <div><p>Koz: with_connection doesn't associate the connection with the thread, so that regular code like Model.find would still pull another connection. That's yet another option to make with_connection stash the connection away in the thread if it isn't there.</p></div> false 2009-02-19T17:04:39+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-19T17:04:41+00:00 17057 Nick Sieger coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 Just seems to me like unless there's a reason that with_connection *shouldn't* do that, then it'd be cleaner than messing with .connection What's the behaviour of this new method if a connection is already checked out, does it check out another and replace all the thread locals? <div><p>Just seems to me like unless there's a reason that with_connection <em>shouldn't</em> do that, then it'd be cleaner than messing with .connection</p> <p>What's the behaviour of this new method if a connection is already checked out, does it check out another and replace all the thread locals?</p></div> false 2009-02-19T19:01:59+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-19T19:02:00+00:00 141 Michael Koziarski coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 Well there are two versions... In the original version (cleanup_connection method), if a connection is already checked out it won't check out a new one. But will release it when the block completes. In the latest version it wouldn't check out a new connection either but also wouldn't release the connection afterwards. I don't have a problem with putting the functionality in with_connection. I agree changing connection method is a heavier change, even though without a block it would function as before. I really don't care where this functionality ends up, but at this point if I had to vote, it seems that modifying the connection method would be cleanest and point people in the right direction in terms of checkins/checkout. <div><p>Well there are two versions...</p> <p>In the original version (cleanup_connection method), if a connection is already checked out it won't check out a new one. But will release it when the block completes.</p> <p>In the latest version it wouldn't check out a new connection either but also wouldn't release the connection afterwards.</p> <p>I don't have a problem with putting the functionality in with_connection. I agree changing connection method is a heavier change, even though without a block it would function as before. I really don't care where this functionality ends up, but at this point if I had to vote, it seems that modifying the connection method would be cleanest and point people in the right direction in terms of checkins/checkout.</p></div> false 2009-02-19T19:10:54+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-19T19:10:58+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 I've noticed that ActiveRecord::Base.connection_pool.with_connection doesn't override the connection for all your models... If you plan to ActiveRecord::Base.connection_pool.with_connection do |conn| u = User.new u.save! end then your user object is not saved with the connection you opened with_connection. Instead you have to do: User.connection_pool.with_connection do |conn| u = User.new u.save! end Is this by design? Why? If we change is_connection to: def with_connection conn = checkout old_conn = @reserved_connections[current_connection_id] @reserved_connections[current_connection_id] = conn yield conn ensure checkin conn @reserved_connections[current_connection_id] = old_conn end Then it works as I had expected it should. <div><p>I've noticed that ActiveRecord::Base.connection_pool.with_connection doesn't override the connection for all your models...</p> <p>If you plan to</p> <p>ActiveRecord::Base.connection_pool.with_connection do |conn|</p> <pre> <code>u = User.new u.save! </code> </pre> <p>end</p> <p>then your user object is not saved with the connection you opened with_connection. Instead you have to do:</p> <p>User.connection_pool.with_connection do |conn|</p> <pre> <code>u = User.new u.save! </code> </pre> <p>end</p> <p>Is this by design? Why?</p> <p>If we change is_connection to:</p> <pre> <code> def with_connection conn = checkout old_conn = @reserved_connections[current_connection_id] @reserved_connections[current_connection_id] = conn yield conn ensure checkin conn @reserved_connections[current_connection_id] = old_conn end </code> </pre> <p>Then it works as I had expected it should.</p></div> false 2009-02-26T16:01:23+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-26T16:01:29+00:00 15613 Jacob Burkhart coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 Yes, the issue you pointed out is exactly why I created this ticket. We need more than what your implementation of with_connection provides. If you have nested calls to with_connection you'll check out a new connection for each one where you really want to be using the same connection throughout all of the calls. <div><p>Yes, the issue you pointed out is exactly why I created this ticket.</p> <p>We need more than what your implementation of with_connection provides. If you have nested calls to with_connection you'll check out a new connection for each one where you really want to be using the same connection throughout all of the calls.</p></div> false 2009-02-26T16:08:04+00:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-02-26T16:08:07+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 1366 6 I've moved the functionality to with_connection per Koz's suggestion. If a connection exists with_connection will simply yield the connection. If a connection doesn't exist with_connection will call connection which will checkout a connection and set it as the current connection, yield it, and ensure it's checked in after the block finishes. This ensures that any nested with_connection calls won't check out multiple connections. ... And it doesn't increase the # of lines of the with_connection method :) <div><p>I've moved the functionality to with_connection per Koz's suggestion.</p> <p>If a connection exists with_connection will simply yield the connection. If a connection doesn't exist with_connection will call connection which will checkout a connection and set it as the current connection, yield it, and ensure it's checked in after the block finishes.</p> <p>This ensures that any nested with_connection calls won't check out multiple connections.</p> <p>... And it doesn't increase the # of lines of the with_connection method :)</p></div> false 2009-03-11T23:06:07+00:00 37814 --- :title: Adding cleanup_connection method 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-03-11T23:06:10+00:00 37814 coderrr coderrr Pratik http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 141 7 false 2009-03-12T00:34:02+00:00 37814 --- :assigned_user: 1366 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-03-12T00:34:04+00:00 1366 Pratik coderrr Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 141 7 So what's the deal? Do you have any response Koz? <div><p>So what's the deal? Do you have any response Koz?</p></div> false 2009-04-29T04:50:41+01:00 37814 --- {} 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 new activerecord connection patch pool 2009-04-29T04:50:46+01:00 37814 coderrr coderrr Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x 141 7 (from [5501b99a19a2a67a9a920fd3c7bff071a2ecf058]) Ensure ActiveRecord::Base.connection_pool.with_connection creates a new connection only when needed [#1752 state:resolved] Signed-off-by: Pratik Naik <pratiknaik@gmail.com> http://github.com/rails/rails/commit/5501b99a19a2a67a9a920fd3c7bff071a2ecf058 <div><p>(from [5501b99a19a2a67a9a920fd3c7bff071a2ecf058]) Ensure ActiveRecord::Base.connection_pool.with_connection creates a new connection only when needed [<a href="/projects/8994/tickets/1752" title="Ticket #1752">#1752</a> state:resolved]</p> <p>Signed-off-by: Pratik Naik <a href="mailto:pratiknaik@gmail.com">pratiknaik@gmail.com</a> <a href="http://github.com/rails/rails/commit/5501b99a19a2a67a9a920fd3c7bff071a2ecf058"> http://github.com/rails/rails/co...</a></p></div> true 2009-05-01T14:58:50+01:00 37814 --- :state: new 9903 1752 added-using_connection-method-like-with_connection-but-sets-it-as-default-connection-while-inside-the-block 0 8994 resolved activerecord connection patch pool 2009-05-01T14:58:51+01:00 17393 Repository coderrr Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1752 2.x fb4179bf45ebf635a92fa3855aed5bafc075a2b4 text/plain 2009-01-13T22:36:38+00:00 using_connection.diff 78080 2921 37814 http://rails.lighthouseapp.com/attachments/78080/using_connection.diff de802c4f48910b48c79b8fe2b0d3417ec2de057e text/plain 2009-01-14T19:58:42+00:00 cleanup_connection.diff 78449 3287 37814 http://rails.lighthouseapp.com/attachments/78449/cleanup_connection.diff da8846150cf32d8cf467ebd685264597d03fa241 text/plain 2009-01-14T22:03:34+00:00 cleanup_connection.diff 78514 3912 37814 http://rails.lighthouseapp.com/attachments/78514/cleanup_connection.diff c524e36e8adcb2bd5bc1cc7511430df504b58ee1 text/plain 2009-01-14T22:05:42+00:00 cleanup_connection.diff 78515 3918 37814 http://rails.lighthouseapp.com/attachments/78515/cleanup_connection.diff 5bdc933f7fbed25ba95592e0c9a651cbdb76f6bb text/plain 2009-02-17T21:31:53+00:00 cleanup_connection.diff 89843 4880 37814 http://rails.lighthouseapp.com/attachments/89843/cleanup_connection.diff 389ca6c7e07f85be838f6e8042095c7f16b1bb92 text/plain 2009-02-18T20:28:48+00:00 0001-Allow-AR-Base.connection-to-take-a-block-meaning-cl.patch 90283 6645 17057 http://rails.lighthouseapp.com/attachments/90283/0001-Allow-AR-Base.connection-to-take-a-block-meaning-cl.patch 0d6878b2888d2473e0a0e942652ca97392c71204 text/plain 2009-03-11T23:06:07+00:00 with_connection.diff 98847 3840 37814 http://rails.lighthouseapp.com/attachments/98847/with_connection.diff