Code review comment for lp:~vila/bzr/525571-lock-bazaar-conf-files

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>
> Overall, I agree that we should continue thinking about which lock model
> we want here, the proposed one is incrementally better and at least
> address the bug better than the minimal one landed for 2.1.
>
> Orthogonal to the lock scope, sharing a config files at the
> wt/branch/repo level could at least avoid reading the config file for
> each variable which could make read locks less painful, but still, I
> don't think it would be enough.

I think defining a scope is reasonable, and just sticking with that.

IMO, configs could be treated just like the rest of the branch data. So
if you '.lock_read()' at the beginning of a process, you read the config
state at that point in time, and then never read the config file again.

When you go to write, you probably need to re-read the file, because a
given text file has more content than what you are likely to be adjusting.

We *could* implement some sort of CAS to make config updates atomic. I
don't think that is worthwhile. (I tried to overwrite parent X with
parent Y, but parent was already Z.)

I agree that you shouldn't try to abuse configs to store incremental
counters, etc.

I honestly don't know what state was getting trashed by concurrent
updates, but I have the feeling that all of the bzr core data doesn't
really care. (Yes, you might get the wrong parent pointer, or submit
pointer, but we don't really store 'live' data in conf files.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwzP7gACgkQJdeBCYSNAANa5QCdFVLPjMHJLyKn7MvR9NdkiAhp
6r8An3u2enhCMDQVbPS/wOmeo5LUO6X3
=Kmd7
-----END PGP SIGNATURE-----