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.

Sign up for GitHub

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

Fix for #230 Error due to non-existing table #235

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix for #230 Error due to non-existing table #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f878ed
Fix for duplicate uniq_agents
mmnelemane Jul 26, 2018
5eeeab4
Fix for #230 to handle nested sessions
mmnelemane Jul 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion aim/db/agent_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Agent(model_base.Base, model_base.HasId, model_base.AttributeMixin):

__table_args__ = (
sa.UniqueConstraint('agent_type', 'host',
name='uniq_agents0agent_type0host'),
name='aim_agents_uniq_agents0agent_type0host'),
model_base.Base.__table_args__
)

Expand Down
2 changes: 1 addition & 1 deletion aim/db/migration/alembic_migrations/versions/accfe645090a_agent_resource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def upgrade():
sa.Column('version', sa.String(10)),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('agent_type', 'host',
name='uniq_agents0agent_type0host'))
name='aim_agents_uniq_agents0agent_type0host'))

op.create_table(
'aim_agent_to_tree_associations',
Expand Down
18 changes: 12 additions & 6 deletions aim/db/migration/alembic_migrations/versions/aceb1ac13668_vmm_policy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
from aim import context
from aim.db import api

from oslo_db.exception import DBError


def upgrade():

Expand All @@ -59,12 +61,16 @@ def upgrade():
new_vmms = []
new_phys = []
with session.begin(subtransactions=True):
for vmm in session.query(old_vmm_table).all():
new_vmms.append(resource.VMMDomain(type=vmm.type, name=vmm.name,
monitored=True))
for phys in session.query(old_phys_table).all():
new_phys.append(resource.PhysicalDomain(name=phys.name,
monitored=True))
try:
for vmm in session.query(old_vmm_table).all():
new_vmms.append(resource.VMMDomain(type=vmm.type, name=vmm.name,
monitored=True))
for phys in session.query(old_phys_table).all():
new_phys.append(resource.PhysicalDomain(name=phys.name,
monitored=True))
except DBError:
# We can fail if the tables are not yet there
pass

op.drop_table('aim_vmm_domains')
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't this fail too if the table didn't exist? Also, why wouldn't the table exist? It is created by a previous revision (1249face3348)

Copy link
Author

Choose a reason for hiding this comment

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

The bug has two aspects:

Problem 1.
As described in the Issue #230, there is a new connection created by aceb1ac13668 before the commit of the earlier transactions started by the previous session. Since these transactions were not committed, the SQL statement creating the table did not commit too. You can refer to the SQL output attached here: https://github.com/noironetworks/aci-integration-module/files/2206403/postgresql.log-201806211158.txt.

Problem 2.
Looking at the logic in aceb1ac13668_vmm_policy.py, it is trying to fetch data from the previous schema and populating into the new schema. This logic could work well for existing table. In fact, the above issue (1) does not affect operation since a table from previous deployment already exists. But for a fresh deployment, the old schema tables do not exist and will be created fresh during migration which is not happening due to (1).

Solution:
The solution in this commit is to ignore failure to read the old tables. If tables do not exist, the queries will result in a DBError exception which we catch and ignore.

Hope this explanation helps.

op.drop_table('aim_physical_domains')
Expand Down