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

Add a connection to the pool if there is only one for embedded ansible #16477

Merged

Conversation

carbonin
Copy link
Member

Before #6786 we used to have one connection specified in the connection pool in database.yml

Installations which have been upgraded from before this change still have that one connection pool in place.

The EmbeddedAnsible worker uses a thread rather than a new process so it shares the connection pool with the server. When the pool is set to only contain one connection, the EmbeddedAnsible worker will not be able to start.

This commit adds a connection to the pool in the same way that we do for workers which specify a specific connection pool size in their settings:

def set_connection_pool_size
cur_size = ActiveRecord::Base.connection_pool.instance_variable_get(:@size)
new_size = worker_settings[:connection_pool_size] || cur_size
return if cur_size == new_size
ActiveRecord::Base.connection_pool.instance_variable_set(:@size, new_size)
_log.info("#{log_prefix} Changed connection_pool size from #{cur_size} to #{new_size}")
end

https://bugzilla.redhat.com/show_bug.cgi?id=1484150

…ble worker

Before ManageIQ#6786 we used to
have one connection specified in the connection pool in database.yml

Installations which have been upgraded from before this change
still have that one connection pool in place.

The EmbeddedAnsible worker uses a thread rather than a new process
so it shares the connection pool with the server. When the pool
is set to only contain one connection, the EmbeddedAnsible worker
will not be able to start.

This commit adds a connection to the pool in the same way that we do
for workers which specify a specific connection pool size in their settings:
https://github.com/ManageIQ/manageiq/blob/f6f7120749d16fd7825f83001dfd875cdecb903c/app/models/miq_worker/runner.rb#L71-L78

https://bugzilla.redhat.com/show_bug.cgi?id=1484150
@miq-bot
Copy link
Member

miq-bot commented Nov 14, 2017

Checked commit carbonin@a3f50f1 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I savagely approve of this surgical change. It's better than visiting "hunert" appliances.

@jrafanie jrafanie merged commit 57b5da5 into ManageIQ:master Nov 15, 2017
@jrafanie jrafanie added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 15, 2017
@carbonin carbonin deleted the alter_connection_pool_for_embedded_ansible branch November 15, 2017 14:26
simaishi pushed a commit that referenced this pull request Nov 15, 2017
…edded_ansible

Add a connection to the pool if there is only one for embedded ansible
(cherry picked from commit 57b5da5)

https://bugzilla.redhat.com/show_bug.cgi?id=1513631
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 7429f75c26c6beaeb37150b7d668fad118794865
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Wed Nov 15 09:07:56 2017 -0500

    Merge pull request #16477 from carbonin/alter_connection_pool_for_embedded_ansible
    
    Add a connection to the pool if there is only one for embedded ansible
    (cherry picked from commit 57b5da5bde3227154d4e1173f28cd324ea266b01)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1513631

simaishi pushed a commit that referenced this pull request Nov 20, 2017
…edded_ansible

Add a connection to the pool if there is only one for embedded ansible
(cherry picked from commit 57b5da5)

https://bugzilla.redhat.com/show_bug.cgi?id=1514139
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit ff449ade007565cc8f5b3d53667b2e0c6bb94bf4
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Wed Nov 15 09:07:56 2017 -0500

    Merge pull request #16477 from carbonin/alter_connection_pool_for_embedded_ansible
    
    Add a connection to the pool if there is only one for embedded ansible
    (cherry picked from commit 57b5da5bde3227154d4e1173f28cd324ea266b01)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1514139

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…_for_embedded_ansible

Add a connection to the pool if there is only one for embedded ansible
(cherry picked from commit 57b5da5)

https://bugzilla.redhat.com/show_bug.cgi?id=1514139
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants