Skip to content

Skip lmdb compat tests #2314

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

Closed
wants to merge 4 commits into from
Closed

Skip lmdb compat tests #2314

wants to merge 4 commits into from

Conversation

IvoDD
Copy link
Collaborator

@IvoDD IvoDD commented Apr 10, 2025

We have seen spuirous very rare segfaults during cleanup of lmdb compat tests.

In general lmdb compat tests are not that useful as not much old data is stored on lmdb. And it is hard to correctly handle lmdb for compat tests which require muliple processes acessing the same LMDB storage (which LMDB doesn't fully support)

Reference Issues/PRs

What does this implement or fix?

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

IvoDD added 4 commits April 9, 2025 17:45
Previously in several places we were calling `old_ac.clear()` to clear
all arctic/libraries created by an compatibility tests.

This however was producing issues when running pytest with
multiprocessing as we do on the CI:
- Each pytest worker sets up its own instance of the session scoped
  fixtures
- When one process finishes with a fixture it runs the cleanup on an
  arctic instance which is potentially actively used by another pytest
worker. Thus the spurious mongo library not found errors.

To mitigate this we introduce a new context manager `CompatLibrary`. It
is responsible for setting up the `Venv`, `VenvArctic` and `VenvLib` and
also creates the library on `__enter__` and cleans the library on `__exit__`.

So we rework all compat tests to use the new `CompatLibrary`
For `test_storage_mover_clone_old_library` we need to create multiple
libraries from the same arctic instance. (because we can't have multiple
active arctic instances to lmdb)
For LMDB on windows deleting a directory will fail if a file in it is
still in use. Likely the short lived venv process or an active arctic
instance remains alive for a while longer and prevents the delete.

In this case we just catch the storage exception and leave the deletion
to the temporary direcory used for the root of lmdb.
We have seen spuirous very rare segfaults during cleanup of lmdb compat
tests.

In general lmdb compat tests are not that useful as not much old data is
stored on lmdb. And it is hard to correctly handle lmdb for compat tests
which require muliple processes acessing the same LMDB storage (which
LMDB doesn't fully support)
Base automatically changed from fix-compat-test-cleanup to master April 10, 2025 13:55
@IvoDD
Copy link
Collaborator Author

IvoDD commented May 14, 2025

LMDB is important to a lot of hobbyists and people first trying out arcticdb. We need to make sure it's a good experience and that includes cross version compatibility so it would be better to not completely drop these.

@IvoDD IvoDD closed this May 14, 2025
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.

2 participants