On Fri, Aug 15, 2014 at 12:15 PM, Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com
> wrote:
> > To be constructive, I implemented the feature in the db/uuid
> > instead of db/format - taking your code and commentary
> > where possible. That automatically covered the dump / load
> > issue and shortened the patch quite a bit. Cache keying
> > and structure description update are also addressed.
> >
> > I did not implement an extension to 'svnadmin setuuid' to only
> > update the instance ID because I'm not sure I really want it.
> >
> > A minor thing I did is to call the new feature 'instance ID'
> > instead of 'instance UUID' because there is no need to guarantee
> > global uniqueness. Our current implementation just happens
> > to generate UUIDs. Future code might use something else.
> > But that is a only a minor point.
>
> Agreed. I like the approach chosen in the V2 patch, so, I did a few
> tweaks to
> it and committed the patch in r1618138.
>
I reviewed the diff between my patch and r1618138 and
agree with your modifications.
> Worth mentioning, I found the "dump / load" part of the log message a bit
> misleading. As I see it, we do not really *need* to bump the instance ID
> when
> going through dump / load procedure. Whenever you load the dump into an
> empty
> destination, that destination will have a unique instance ID, and nothing
> bad
> will happen even if that instance ID survives through the svn_fs_set_uuid()
> call. However, I am fine with also bumping the instance ID here, because
> it
> is a bit simpler to implement and kind of fits the whole concept.
Well, currently there is no reason (such like keeping caches
valid etc.) to keep an instance ID once you bump the UUID.
Instance IDs are an internal aspect of the implementation and
we are free to change their usage / preservation scheme in
later releases.
> Presumably,
> it is also better in two edge cases that I could think of (from my point of
> view, both of them are "broken workflows", but they are still theoretically
> possible):
>
> - Recovering from a disaster by loading an incremental dump into an old
> repository snapshot with '--force-uuid', and performing a hot swap after
> it.
>
> - Loading identical dumps (or different dumps, but with '--ignore-uuid')
> into an
> empty template repository, e.g., a repository with configured hooks and
> common
> access rights. This workflow assumes that the template repository is
> being
> copy-pasted prior to loading something into it.
>
> So, I tweaked this part of the log message accordingly. Hopefully, I am
> not
> missing something crucial here. I would also like to solve the problem
> with
> incremental hotcopy not taking the pack-lock on the destination (as
> mentioned
> earlier), because now this should be fairly trivial.
>
Here at the SHF hackathon, we briefly talked about how to
proceed. I quite happy with the current implementation wrt
to its technical approach. However, we identified another
operation that should bump the instance ID, fs_recover.
And there might be more.
None of that is complicated to solve in any way but we should
have a full, reviewed list of places that need to update or
possibly use the instance ID before releasing it. So, the
correct way to to that is having a branch for the full feature -
as suggested by Brane.
Given that many of us are the same room for this week,
progress on that branch as well as its review can be quick.
Unless there is something fundamentally broken with it,
there is a good chance to have it in 1.9.
> P.S. I have reread my previous answers in this thread and I realized that
> they
> can be considered quite arrogant.
:D To me, you came across as fighting desperately to get
the change into the 1.9 release. More specifically, you seemed
to have a hard time seeing any reason not to just apply that
simple enough fix to for specific problem.
> That was utterly unnecessary (it never *is*
> necessary, actually), so, I am terribly sorry for that.
>
I really appreciate this. Written communication is the most
lossy, opening things to bias and lots of second guessing.
I have to remind myself of that fact once in a while.
Cheers!
-- Stefan^2.
Received on 2014-08-18 16:35:31 CEST