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

Session issues with DbWorkflowData (in SQLA) #883

Closed
szoupanos opened this issue Nov 1, 2017 · 2 comments
Closed

Session issues with DbWorkflowData (in SQLA) #883

szoupanos opened this issue Nov 1, 2017 · 2 comments
Assignees
Milestone

Comments

@szoupanos
Copy link
Contributor

There is a problem with DbWorkflowData that can be used to store Nodes (DbNodes) in a script that we have with Leo (@lekah) to test old workflows under SQLA (attached) using N. Mounet's workflows (@nmounet).

The problem appears when someone adds an attribute to a workflow as follows:

wf = WFTestSimpleWithSubWF()
wf.store()
wf.add_attribute('a', dbnode)

when there is in the session a previous version of the object dbnode.

During the execution of the script, we get an error message like the following:

A conflicting state is already present in the identity map for key (<class 'aiida.backends.sqlalchemy.models.node.DbNode'>, (254,))

After a lot of search, I managed to reproduce the problem but I don't have the time to locate the source of it (could be also in Nicola's script). I don't know if it matters also a lot.

One of the indirect problems seems to be the "dirty" way that we use the sessions: we continuously add objects and, if I am not mistaken, we never remove them. This is not too bad, since we have an expire_on_commit=True sessions, meaning that after a commit, the database is consulted to for every attribute/object access.
http://docs.sqlalchemy.org/en/latest/orm/session_api.html

However, In our case, it seems that for some reason, an older version of the object dbnode should be, somehow, added to the session. Then, during the

wf.add_attribute('a', dbnode)

dbnode is automatically added to the session when the set_value() method of DbWorkflowData is called since there is a relationship between the dbnodes and DbWorkflowData.

The automatic addition is not a bad thing. The problem is that for some reason that I don't know, an older (and expired) version of dbnode was in the session. So when the self.save() line of the set_value() method was executed, we got the aforementioned error message.

What can be done is to refresh the objects used in the session and getting a fresh copy of dbnode in the set_value method. So instead of doing

self.aiida_obj = arg.dbnode

we should do

self.aiida_obj = sess.merge(arg.dbnode, load=True)

where merging does some kind of "synchronization" of the arg.dbnode with the instance existing in the database and updates the existing session object.
http://docs.sqlalchemy.org/en/latest/orm/session_state_management.html#merging

Here is a script that demonstrates the problem.

from aiida.orm.node import Node
from aiida.workflows.test import WFTestSimpleWithSubWF
from aiida.backends.sqlalchemy import get_scoped_session
from aiida.backends.sqlalchemy.models.node import DbNode

# Create a node and store it
n = Node()
n.store()

# Keep some useful information
n_id = n.dbnode.id
old_dbnode = n.dbnode

# Get the session
sess = get_scoped_session()
# Remove everything from the session
sess.expunge_all()

# Create a workflow and store it
wf = WFTestSimpleWithSubWF()
wf.store()
 
n_reloaded = load_node(n_id)
sess.add(old_dbnode)

wf.add_attribute('a', n_reloaded)

I suppose that this problem interests also @giovannipizzi
I will make a pull request that corrects the WFData behaviour and also adds the test shown above.

run_test.sh.txt
generate_pwworkflow_params.py.txt

@szoupanos szoupanos added this to the v0.11.0 milestone Nov 1, 2017
@szoupanos szoupanos self-assigned this Nov 1, 2017
@szoupanos
Copy link
Contributor Author

szoupanos commented Nov 1, 2017

This is an interesting page that mentions the same (or very similar) problem
https://groups.google.com/forum/#!topic/sqlalchemy/n_OK8HSfxwo

@szoupanos
Copy link
Contributor Author

Fix with #889

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

No branches or pull requests

1 participant