Re: Pristine text missing - cleanup doesn't work

Bert Huijben wrote:
>>>  * TODO: At least check that any zero refcount is really correct, before
>>>  *       using it.
>>
>>  We already do this in debug builds.

Hmm, you're right, when using a current 1.8-dev build and a fresh WC.

When I tested this scenario the other day it was in an old WC that has been upgraded from time to time, and is now supposedly at 1.8.  There (I just repeated to make sure) if I set the refcount to 0, then 'svn cleanup' quietly deletes the 'pristine' table row and the file.

The DB schema in that WC does not match what is should be for 1.8 -- it doesn't have any 'REFERENCES PRISTINE (checksum)' clauses in it.  It may have got that way by being upgraded incrementally through several 1.7-era development versions, so we need not be too concerned about it.

(While investigating, I discovered and filed issue #4358 "Svn WC 1.8 upgrade from 1.7 - wrong schema", <http://subversion.tigris.org/issues/show_bug.cgi?id=4358>.  The schema differences there do *not* include a missing 'REFERENCES' clause.)

I tested with a checkout containing just one file in four freshly created WCs:

- checkout with 1.6.20, upgrade with 1.8.x
- checkout with 1.6.20, upgrade with 1.7.8, upgrade with 1.8.x
- checkout with 1.7.8, upgrade with 1.8.x
- checkout with 1.8.x

and in each of these, I ran:

- sqlite3 'update pristine set refcount=0'
- svn cleanup  # fails: 'foreign key constraint failed'
- svn cat foo  # succeeds
- sqlite3 'update pristine set refcount=1'
- svn cleanup  # succeeds

In each case, 'svn cleanup' errored out with an 'foreign key constraint failed' error, preventing any further damage.

So that seems to be fairly safe when the foreign key constraint checking is enabled.

>>  The foreign key check enforces this, but at a certainly not-null performance
>>  cost (understatement). This is why I haven't enabled this for release builds.
>>  (The original reason for not enabling them was that it required a newer Sqlite
>>  version)
>>
>>  You can't delete a PRISTINE row that is used with foreign keys enabled. (That
>>  part is currently not backed by an index... the performance killer. And
>>  optimized away by our reference counting via triggers)
>
> If we perform a check to verify the reference count before using it for cleanup
> we should just remove the reference count updating triggers. This gives a few
> percent performance boost to every update/add/delete on the NODES table.

If we removed the ref-count-updating triggers, then we wouldn't be able to select pristines for deletion based on refount==0.  What would we do instead?

> Given that we never update the reference count manually I would be very
> surprised if we could ever find a reason why somebody not using the Sqlite db
> directly (via Sqlite or a third party sqlite db implementation) would get the
> refcount in a broken state.

Well, it has happened before and it will probably happen again.

- Julian
Received on 2013-04-22 19:28:30 CEST