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

Environment.set_mapsize() safety #96

Closed
dw opened this issue Aug 11, 2015 · 5 comments
Closed

Environment.set_mapsize() safety #96

dw opened this issue Aug 11, 2015 · 5 comments
Assignees

Comments

@dw
Copy link
Collaborator

dw commented Aug 11, 2015

Ensure that cached read-only transactions do not break when set_mapsize() is invoked.

@dw
Copy link
Collaborator Author

dw commented Aug 11, 2015

Ensure write transactions are synchronized, see #97.

class Environment:
    env_change_in_progress = False
    env_change_complete_event = threading.Event()

    def begin():
        if env_change_in_progress:
            env_change_complete_event.wait()

    def set_mapsize(..., timeout=30.0):
        env_change_in_progress = True
        while True:
            if time() > timeout:
                 raise TimeoutError
            if not any_deps_are_active_txns():
                 break
            sleep(0.500)
         resize_map()
         env_change_in_progress = False
         env_change_complete_event.notify()

@jnwatson
Copy link
Owner

I'm going to fix this. I have a solid crashing unit test for txn.get/set_mapsize race, a txn.put/set_mapsize race, and a env.close/txn.get race. I like the general idea of keeping the overhead low, but there's still a place after the begin checks for env_change_in_progress and before the active txns list is updated where a transaction user isn't locked out.

The problem is a bit worse on the close side. The close function needs to wait for all other transactions and cursors to complete their function.

How would do you propose to implement the any_deps_are_active_txns() function?

@jnwatson
Copy link
Owner

Upon further thought, I think the right approach is to increment a in_use count almost every method on entrance and decrement every method on exit, and have every method start with a env_change_in_progress check. set_mapsize and close would loop waiting for in_use to drop to 0. The GIL would protect any races with the increment/change check.

@dw
Copy link
Collaborator Author

dw commented May 13, 2018 via email

@jnwatson
Copy link
Owner

Well, I fixed it in a branch, but it added a bunch of code and overhead for every call. (It is not unlike the many unfruitful attempts to remove the GIL from cpython).

Unfortunately, I think the correct approach for now is to document the behavior and kick the can to the user.

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

No branches or pull requests

2 participants