GNOME Bugzilla – Bug 707644
Move ref writing to be transaction-based
Last modified: 2013-09-07 00:33:58 UTC
This is something Colin and I talked about yesterday. It doesn't make ref writing atomic (since we write a bunch of different files), but it will help with that. I haven't talked it over with Colin yet, but I think it makes to store all refs one file, like the summary file, and simply parse/write that, rather than having each ref be a different file in the filesystem.
Created attachment 254276 [details] [review] builtin-refs: Remove the ref delete function This should be part of write-refs
Created attachment 254277 [details] [review] repo: Make the ordering consistent between abort/complete_transaction
Created attachment 254278 [details] [review] repo: Make OSTreeCommitModifier introspectable
Created attachment 254279 [details] [review] repo: Make prepare_transaction introspectable
Created attachment 254280 [details] [review] repo: Move commit code to another file
Created attachment 254281 [details] [review] repo: Rename "stage" to "write" in the API
Created attachment 254282 [details] [review] repo: Move the transaction stats to a separate struct This is much easier for callers to handle, and simplifies the API a lot.
Created attachment 254283 [details] [review] repo: Make abort_transaction silently succeed if we're not in a transaction This helps callers out a lot, and means we can always call abort_transaction at the end of a function.
Created attachment 254284 [details] [review] Move ref writing to be transaction-based Rather than having separate write_ref calls, make clients start a transaction, add some refs, and then commit it. While this doesn't make it 100% atomic, it makes it easier for us to use an atomic model, and it means we don't do as much I/O updating the summary file and such.
Review of attachment 254276 [details] [review]: I'd rather kill off write-refs; "refs" is a bit more general purpose, just like "git tag" allows you to both create and delete.
Review of attachment 254277 [details] [review]: LGTM
Review of attachment 254278 [details] [review]: Right.
Review of attachment 254279 [details] [review]: Yep.
Review of attachment 254280 [details] [review]: Makes sense.
Review of attachment 254281 [details] [review]: Definitely.
Review of attachment 254282 [details] [review]: OK.
Review of attachment 254283 [details] [review]: Feels a bit weird to have abort at the end of every function, but OK.
Review of attachment 254284 [details] [review]: Need to update gnome-ostree for this right? Maybe instead of _write_ref call it _set_ref() since we aren't actually writing at that point? Otherwise looks good to me.
pushed all of these
well, except the one that deletes builtin-refs. also made other api cleanups and applied review comments