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

Avoid locking database when importing properties in a MPI session #181

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

Martin-Rey
Copy link
Contributor

This suggestion from @apontzen fixes #180.

Is there a good way to test for this parallel-only behaviour? I could find some tests where the PropertyImporter is used in an integrated way (test_ahf_trees, test_stat_files), but it does not seem like there is a dedicated test suite for its internal capabilities.

@apontzen
Copy link
Member

It would be useful to have a clearer test suite for PropertyImporter. As for parallel issues, they are notoriously hard to test unfortunately. The underlying locking mechanisms are tested in test_parallel_tasks, which is hard enough. If you can come up with a neat and reliable way to test the integrated behaviour robustness to SQLite locks that would be great, but I don't think it's an easy task.

@Martin-Rey
Copy link
Contributor Author

Ok, I wouldn't know where to start on parallel testing unfortunately.

As for a more general test suite for the PropertyImporter, I have an unfinished PR upgrading its capabilities to address #162. Maybe it makes more sense to add the test suite there when we are actually developing the class, rather than here?

@apontzen
Copy link
Member

Yes, I'm happy with doing it that way

@Martin-Rey
Copy link
Contributor Author

Ok, then happy to merge once the tests have finished (is it usual for them to take that long now?)

@apontzen
Copy link
Member

Yes there is now a full integration test that takes about 1hr to run

@apontzen apontzen merged commit b828357 into pynbody:master Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Systematic database lock when importing first properties with MPI
2 participants