[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r959257 - /subversion/trunk/subversion/libsvn_wc/wc_db_pdh.c

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Thu, 01 Jul 2010 10:08:32 +0100

Greg Stein <gstein_at_gmail.com> writes:

> On Wed, Jun 30, 2010 at 13:55, Philip Martin <philip.martin_at_wandisco.com> wrote:
>> Greg Stein <gstein_at_gmail.com> writes:
>>
>>> I completely disagree with this.
>>>
>>> Didn't I just get done describing the intent here?
>>
>> You didn't respond to my questions in the last thread.
>
> Gee. I'm sorry. I have a life. I don't have time to respond to
> everything RIGHT NOW. Maybe I needed to think about it? Maybe I'm
> traveling. Maybe I just want to sit back and play Mass Effect 2 for a
> while.

I realise that. You seemed to be accusing me of ignoring you, and I
was pointing out that I didn't ignore you it's just that I don't agree
with you. Sorry if it came across the wrong way.

>>> We allow the
>>> upgrade to continue. If there is an impact on work queue items, then
>>> they must be upgraded, too.
>>
>> We don't generally support running old workqueues with newer code, so
>> why is uprade so different?  Take this recent commit:
>>
>>  http://svn.apache.org/viewvc?rev=959374&view=rev
>>
>> it removes SVN_WC__LOG_DELETE_LOCK.  Every time we remove one of the
>> log actions we effectively broke any workqueue that included them.
>> Nobody has written any code to migrate those workqueues.
>
> Yup. And because that would only affect devs, and when the workqueue
> code throws an error saying "dunno what 'delete-lock' means", then
> that dev has an easy way to fix themselves. It is an obvious failure
> mode.
>
> I don't care about breaking devs. The interim formats before 1.7 is
> released can enforce a cleanup first. But even that is a pain. I'd
> rather just see something fail in an obvious way and let the dev fix
> it up.

For devs it's better not to autoupgrade. Then they can revert to the
version that generated the workqueue and use that. If we autoupgrade
they have no usable version.

> BUT: the need-clean-wq-for-upgrade is NOT true moving forward from 1.7.
>
>>> Clients are not expected to back up to previous revisions and run "svn
>>> cleanup". We ONLY plan to require that for the 1.6 -> 1.7 transition.
>>>>From then on, we can/should/will perform an upgrade with a non-empty
>>> work queue. Every single auto-upgrade in the past has operated in this
>>> fashion, and we will continue to do so from 1.7 onwards.
>>>
>>> Please revert. This is contrary to the design that I explained just
>>> yesterday.
>>
>> Why is it so important to support this?  It doesn't affect many people
>> but writing the code to upgrade log items is a big effort.  And it's
>
> We don't have to upgrade today's workqueue items. We simply have to
> have a policy to upgrade them once 1.7 is released.

I don't see the difference. At some point you have to write the code
to do it.

> The process that I
> encoded in the db-opening logic was to support THAT purpose.
>
>> not code that will get used by anyone other than people running dev
>> code.  It will be poorly tested and a maintenance burden.  How long do
>> we keep it?  How far back in wcng history do we support?
>>
>> Take this workqueue item:
>>
>> 13|(loggy 30 /home/pm/sw/subversion/obj/wc1 68 <mv
>>   dest=".svn/text-base/f.svn-base"
>>   name=".svn/svn-ZIxV3z"/>
>> )
>>
>> It will break the working copy on upgrade because the wrong text-base
>> will get copied into the pristine store.
>
> Right. Let it break.
>
> At 1.7 release time, the above style of wq item could be hard to
> upgrade, so we could make it more semantic-based. "install TEMP into
> pristine store as CHECKSUM". That would be much more resilient to
> changes in the working copy format.

But your plan is that at release we should be capable of handling that
old workqueue item? This is a workqueue item only ever produced by a
dev version. I'd argue that these pre-1.7 dev workqueues don't need
to be supported long-term. You realise that the XML parser in log.c
would have to be retained, probably including bits that have already
been deleted? And that parser would never be used by people upgrading
from 1.6, since they have to cleanup first.

>>  When we upgrade and run the
>> workqueue it will probably fail because the .svn/text-base no longer
>> exists.  How will the user handle that?  Isn't it better to get an
>> error saying "cleanup first"?
>
> Nope. Users cannot generally downgrade their client to run a cleanup.
> Historically, we have always auto-upgraded the working copies, even
> with stale logs in them.
>
> The 1.7 upgrade process is too invasive and time-consuming, so we
> decided to have a manual process ('svn upgrade'). But moving forward,
> the intent is to continue an auto-upgrade mechanism even with stale
> workqueues.

After we release 1.7 it's different. We can choose to autoupgrade, or
we can treat it like 1.6 and say that an upgrade step is required.
It's hard to predict the future. If 1.8 uses a totally different
backend for the wc then while we would support upgrading from a 1.7 wc
we might not support running 1.7 workqueues.

>> That's just one example.  I don't know exactly which workqueue items
>> break.  SVN_WC__LOG_CP_AND_TRANSLATE probably breaks.  There could be
>> ones that are no longer in log.c that break, as well as non-loggy
>> ones.
>
> Yup. And you'll also note Bert's latest change to the postcommit
> workqueue item. Before fetching the "no_unlock" parameter from the
> stored skel, he first checked whether it exists. If that portion of
> the skel does not exist (stale item!), then it just assumes
> no_unlock=TRUE.

I'd argue that's unneccesary, it's a maintenance burden we don't need.
Is there a regression test? Does it still work? However it's trivial
compared to supporting an XML parser that's not otherwise required.

> Another approach is to rename the work item in the OP_FOO symbols.
> Stale items become obvious when the new code cannot run them.
>
> I hope that clarifies things, and explains why this change should be
> reverted. I understand your desire to avoid upgrading stale work
> items. We don't have to do that *today*, but we need to plan for it in
> the future, and organize our code/process for that.

I still disagree. Having made the decision to introduce an upgrade
step for 1.6-to-1.7 we should take full advantage of that. The
intermediate pre-1.7 workqueues do not need to be supported. Then we
can throw out all the log parsing code.

-- 
Philip
Received on 2010-07-01 11:09:26 CEST

This is an archived mail posted to the Subversion Dev mailing list.