-
Notifications
You must be signed in to change notification settings - Fork 5
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
Repo locking API backport #184
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Doing anything even somewhat sophisticated requires this; turns out our own `ostree prune` CLI wants this, e.g. ostreedev/ostree#2337 Closes: ostreedev/ostree#2286 (cherry picked from commit 0f36d8c) The new symbols have been added to the released symbols file rather than the development symbols file as in the upstream PR since our package does a released build. Similarly, the upstream `LIBOSTREE_2021.3` symbol version has been used so that packages don't need to be rebuilt when we get to that version and drop our backported commits. https://phabricator.endlessm.com/T31868
This simplifies the lock state management considerably since the previously pushed type doesn't need to be tracked. Instead, 2 counters are kept to track how many times each lock type has been pushed. When the number of exclusive locks drops to 0, the lock transitions back to shared. (cherry picked from commit c3ada6f)
This will allow usage of `GMutexLocker`. This should be available on many older distros: * RHEL 7 - 2.56.1 * RHEL 8 - 2.56.4 * Debian 9 stretch (oldstable) - 2.50.3 * Debian 10 buster (stable) - 2.58.3 * Ubuntu 16.04 xenial - 2.48.2 * Ubuntu 18.04 bionic - 2.56.4 (cherry picked from commit eb09207)
Previously each thread maintained its own lock file descriptor regardless of whether the thread was using the same `OstreeRepo` as another thread. This was very safe but it made certain multithreaded procedures difficult. For example, if a main thread took an exclusive lock and then spawned worker threads, it would deadlock if one of the worker threads tried to acquire the lock. This moves the file descriptor from thread local storage to the `OstreeRepo` structure so that threads using the same `OstreeRepo` can share the lock. A mutex guards against threads altering the lock state concurrently. Fixes: #2344 (cherry picked from commit ccef978)
Use `g_error` and `g_assert*` rather than `g_return*` when checking the locking preconditions so that failures result in the program terminating. Since this code is protecting filesystem data, we'd rather crash than delete or corrupt data unexpectedly. `g_error` is used when the error is due to the caller requesting an invalid transition like attempting to pop a lock type that hasn't been taken. It also provides a semi-useful message about what happened. (cherry picked from commit 89f4ce2)
If there's a locking issue in this test, then it's likely not going to resolve after a few seconds of serializing access. Lower the default 30 second lock timeout to 5 seconds to prevent the test from hanging unnecessarily. (cherry picked from commit 055b263)
The semantics of multiple process locking are covered by test-concurrency.py, but the semantics of the repository locking from a single process aren't handled there. This checks how the repository locking is handled from a single thread with one OstreeRepo, a single thread with multiple OstreeRepos, and multiple threads sharing an OstreeRepo. (cherry picked from commit 06bb56b)
pwithnall
approved these changes
Jun 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backport of ostreedev/ostree#2348 to make the repo locking API public. The symbol changes have been made in the released symbols file since we do released builds. Likewise, the upstream symbol version is used for the new API so that packages don't need to be rebuilt when we get that release.
https://phabricator.endlessm.com/T31868