lib/repo: Add MT support for transaction_set_ref(), clarify MT rules #1358
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
For rpm-ostree I'd like to do importing in parallel with threads; the code is *almost* ready for that except today it calls `ostree_repo_transaction_set_ref()`. Looking at the code, there's really a "transaction" struct here, not just stats. Let's lift that struct out, and move the refs into it under the existing lock. Clarify the documentation around multithreading for various functions.
cgwalters
force-pushed
the
txn-set-ref-lock
branch
from
1319401 to
1bf3940
Compare
cgwalters
added a commit
to cgwalters/rpm-ostree
that referenced
this pull request
Dec 1, 2017
Depends: ostreedev/ostree#1358 For jigdo ♲📦 in order to get true image speed like libostree has we need to interleave and parallelize downloading and importing. The messy part about this is having sync API do the "invoke and wait on various async tasks" pattern. It's the same thing in `ostree_repo_pull_with_options()`. Importing is pretty dramatically faster with this, I can only imagine the speed win if we actually interleaved with downloads. However doing that requires libdnf/librepo work.
jlebon
reviewed
Dec 4, 2017
src/libostree/ostree-repo-commit.c
Outdated
| * ostree_repo_commit_transaction(); that function takes care of writing all of | ||
| * the objects (such as the commit referred to by @checksum) before updating the | ||
| * refs. If the transaction is instead aborted with | ||
| * ostree_repo_abort_transaction(), no changes to the ref be made to the |
src/libostree/ostree-repo-commit.c
Outdated
| * | ||
| * Note however that currently writing *multiple* refs is not truly atomic; if | ||
| * the process or system is terminated during | ||
| * `ostree_repo_commit_transaction()`, it is possible that just some of the refs |
There was a problem hiding this comment.
Minor: seems like the convention is to not backtick-escape function names in documentation, right?
There was a problem hiding this comment.
gtk-doc should support both, but yeah let's be consistent in not doing it for now.
| @@ -1573,9 +1584,10 @@ ostree_repo_transaction_set_refspec (OstreeRepo *self, | |||
| { | |||
There was a problem hiding this comment.
Missing multithreading advisory for this one?
|
This one can wait until after #1359 |
|
OK release done, see fixup ⬆️ above |
|
☀️ Test successful - status-atomicjenkins |
cgwalters
added a commit
to cgwalters/rpm-ostree
that referenced
this pull request
Dec 4, 2017
Depends: ostreedev/ostree#1358 For jigdo ♲📦 in order to get true image speed like libostree has we need to interleave and parallelize downloading and importing. The messy part about this is having sync API do the "invoke and wait on various async tasks" pattern. It's the same thing in `ostree_repo_pull_with_options()`. Importing is pretty dramatically faster with this, I can only imagine the speed win if we actually interleaved with downloads. However doing that requires libdnf/librepo work.
rh-atomic-bot
pushed a commit
to coreos/rpm-ostree
that referenced
this pull request
Dec 5, 2017
Depends: ostreedev/ostree#1358 For jigdo ♲📦 in order to get true image speed like libostree has we need to interleave and parallelize downloading and importing. The messy part about this is having sync API do the "invoke and wait on various async tasks" pattern. It's the same thing in `ostree_repo_pull_with_options()`. Importing is pretty dramatically faster with this, I can only imagine the speed win if we actually interleaved with downloads. However doing that requires libdnf/librepo work. Closes: #1124 Approved by: jlebon
cgwalters
added a commit
to cgwalters/rpm-ostree
that referenced
this pull request
Dec 7, 2017
Depends: ostreedev/ostree#1358 For jigdo ♲📦 in order to get true image speed like libostree has we need to interleave and parallelize downloading and importing. The messy part about this is having sync API do the "invoke and wait on various async tasks" pattern. It's the same thing in `ostree_repo_pull_with_options()`. Importing is pretty dramatically faster with this, I can only imagine the speed win if we actually interleaved with downloads. However doing that requires libdnf/librepo work.
rh-atomic-bot
pushed a commit
to coreos/rpm-ostree
that referenced
this pull request
Dec 7, 2017
Depends: ostreedev/ostree#1358 For jigdo ♲📦 in order to get true image speed like libostree has we need to interleave and parallelize downloading and importing. The messy part about this is having sync API do the "invoke and wait on various async tasks" pattern. It's the same thing in `ostree_repo_pull_with_options()`. Importing is pretty dramatically faster with this, I can only imagine the speed win if we actually interleaved with downloads. However doing that requires libdnf/librepo work. Closes: #1124 Approved by: jlebon
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.
For rpm-ostree I'd like to do importing in parallel with threads; the
code is almost ready for that except today it calls
ostree_repo_transaction_set_ref().Looking at the code, there's really a "transaction" struct here,
not just stats. Let's lift that struct out, and move the refs
into it under the existing lock.
Clarify the documentation around multithreading for various functions.