zodb 3.8.3 strip_versions missing some versions?

[Chris Withers]

Hi All,

I'm upgrading a Zope 2.9.8 project to Zope 2.12 + zeoraid.

As I've done with previous projects, I run the .fs files through zodb
3.8.3's strip_versions script. For both the storages in this project,
the script found no version records.

As a first step, I'm currently running the 2.9.8 app server against a
pair of zeo storage servers running zodb 3.9.3 and zeoraid 1.0b3

However, when I try and view the history tab of *any* page template
within the .fs files, even a newly created one, I get an assertion error
from zeoraid as gocept.zeoraid.storage.RAIDStorage's history method is
getting called with version passed as an integer 20.

Anyone have any idea what that 20 means and why it's being passed like that?

cheers,

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Christian Theune-2]

On 11/02/2009 10:04 AM, Chris Withers wrote:

> Hi All,
>
> I'm upgrading a Zope 2.9.8 project to Zope 2.12 + zeoraid.
>
> As I've done with previous projects, I run the .fs files through zodb
> 3.8.3's strip_versions script. For both the storages in this project,
> the script found no version records.
>
> As a first step, I'm currently running the 2.9.8 app server against a
> pair of zeo storage servers running zodb 3.9.3 and zeoraid 1.0b3
>
> However, when I try and view the history tab of *any* page template
> within the .fs files, even a newly created one, I get an assertion error
> from zeoraid as gocept.zeoraid.storage.RAIDStorage's history method is
> getting called with version passed as an integer 20.
>
> Anyone have any idea what that 20 means and why it's being passed like that?

There's a size argument following the version which probably is not
being passed as a keyword argument so it looks like a slight API
incompatibility. I wonder whether ZEORaid should hack around that or
whether the caller should be changed.

Christian

--
Christian Theune · ct@...
gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany
http://gocept.com · tel [phone] · fax [phone]
Zope and Plone consulting and development

_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Christian Theune wrote:
> There's a size argument following the version which probably is not
> being passed as a keyword argument so it looks like a slight API
> incompatibility. I wonder whether ZEORaid should hack around that or
> whether the caller should be changed.

Hmm... looking more closely at the traceback, the caller is:

   File "ZODB3-3.9.3-py2.6-linux-i686.egg/ZEO/StorageServer.py", line
1382, in history
     return self.storage.history(oid, size)

Now, ZODB 3.9 doesn't support versions, which is why the versions
parameter has vanished, I guess?

However, I wonder if maybe this call should be:

self.storage.history(oid, size=size)

...to support storages which still accept a version parameter *and*
newer ones that don't?

Looking at RelStorage:
http://svn.zope.org/relstorage/trunk/relstorage/storage.py?rev=105120&view=auto
...I can see it's going to have the same problem.

Would anyone object if I wrote a test and fixed this on the 3.9 branch
of ZODB? I'm not really sure what to do about the trunk...

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Christian Theune-2]

On 11/02/2009 10:47 AM, Chris Withers wrote:

> Christian Theune wrote:
>> There's a size argument following the version which probably is not
>> being passed as a keyword argument so it looks like a slight API
>> incompatibility. I wonder whether ZEORaid should hack around that or
>> whether the caller should be changed.
>
> Hmm... looking more closely at the traceback, the caller is:
>
>   File "ZODB3-3.9.3-py2.6-linux-i686.egg/ZEO/StorageServer.py", line
> 1382, in history
>     return self.storage.history(oid, size)
>
> Now, ZODB 3.9 doesn't support versions, which is why the versions
> parameter has vanished, I guess?
>
> However, I wonder if maybe this call should be:
>
> self.storage.history(oid, size=size)
>
> ...to support storages which still accept a version parameter *and*
> newer ones that don't?
>
> Looking at RelStorage:
> http://svn.zope.org/relstorage/trunk/relstorage/storage.py?rev=105120&view=auto
>
> ...I can see it's going to have the same problem.
>
> Would anyone object if I wrote a test and fixed this on the 3.9 branch
> of ZODB? I'm not really sure what to do about the trunk...

I think that's the best idea. It's not exactly part of the policy of
supporting old protocols but if we want to support and encourage 3rd
party storage implementations we should leverage this simple fix for
better backwards compatibility.

Christian

--
Christian Theune · ct@...
gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany
http://gocept.com · tel [phone] · fax [phone]
Zope and Plone consulting and development
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Christian Theune wrote:
>> Would anyone object if I wrote a test and fixed this on the 3.9 branch
>> of ZODB? I'm not really sure what to do about the trunk...
>
> I think that's the best idea. It's not exactly part of the policy of
> supporting old protocols but if we want to support and encourage 3rd
> party storage implementations we should leverage this simple fix for
> better backwards compatibility.

Hmmm, I'm less sure now. The problematic code is here:

http://zope3.pov.lt/trac/browser/ZODB/branches/3.9/src/ZEO/StorageServer.py#L1379

It looks like ZEOStorage308Adapter is used when the protocol specified
is less than Z309, which it is in my case. But, the assumption here is
that the storage being served by StorageServer effectively matches the
Z309 API. zeoraid's RAIDStorage does not...

However, I can't see what `history` method is called if the protocol >=
Z309. Christian, I'm guessing you guys have used zeoraid with 3.9 from
some time now, does this issue not occur if everything is talking Z309?
If not, why not?

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Jim Fulton]

I'm fine with change the storage server to pass the history size
argument as a keyword parameter.

Jim

On Mon, Nov 2, 2009 at 6:04 AM, Chris Withers <chris@...> wrote:

> Christian Theune wrote:
>>> Would anyone object if I wrote a test and fixed this on the 3.9 branch
>>> of ZODB? I'm not really sure what to do about the trunk...
>>
>> I think that's the best idea. It's not exactly part of the policy of
>> supporting old protocols but if we want to support and encourage 3rd
>> party storage implementations we should leverage this simple fix for
>> better backwards compatibility.
>
> Hmmm, I'm less sure now. The problematic code is here:
>
> http://zope3.pov.lt/trac/browser/ZODB/branches/3.9/src/ZEO/StorageServer.py#L1379
>
> It looks like ZEOStorage308Adapter is used when the protocol specified
> is less than Z309, which it is in my case. But, the assumption here is
> that the storage being served by StorageServer effectively matches the
> Z309 API. zeoraid's RAIDStorage does not...
>
> However, I can't see what `history` method is called if the protocol >=
> Z309. Christian, I'm guessing you guys have used zeoraid with 3.9 from
> some time now, does this issue not occur if everything is talking Z309?
> If not, why not?
>
> Chris
>
> --
> Simplistix - Content Management, Batch Processing & Python Consulting
>            - http://www.simplistix.co.uk
> _______________________________________________
> For more information about ZODB, see the ZODB Wiki:
> http://www.zope.org/Wikis/ZODB/
>
> ZODB-Dev mailing list  -  ZODB-Dev@...
> https://mail.zope.org/mailman/listinfo/zodb-dev
>

--
Jim Fulton
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Jim Fulton wrote:
> I'm fine with change the storage server to pass the history size
> argument as a keyword parameter.

Okay, but where is the Z309 equivalent of ZEOStorage308Adapter's history
method?

cheers,

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Chris Withers wrote:
> Jim Fulton wrote:
>> I'm fine with change the storage server to pass the history size
>> argument as a keyword parameter.
>
> Okay, but where is the Z309 equivalent of ZEOStorage308Adapter's history
> method?

Ug, the answer is much less pleasant :-(

So, a ZODB 3.9.3 ClientStorage calls the ServerStub's history method,
this does an rpc call:

self.rpc.call('history', oid, length)

Which comes in on the server and gets despatched straight out in:

/ZEO/zrpc/connection.py", line 581, in handle_request

This, of course, means we can't pull the keyword parameters trick, since
ZEO's rpc doesn't support keyword parameters from what I saw...

I'm not sure what to suggest here...

- release a new version of zeoraid targeted at zodb 3.9+ that doesn't
include any version parameters anywhere. (I dunno how Christian would
feel about the maintenance burden)

- drop support for zodb < 3.9 in zeoraid and remove all version
parameters. That'd be fine by me, dunno how Christian would feel.

- use some kind of StorageInterface to indicate what parameters a
storage expects. (this would require work on both zeoraid and zeoraid :-/)

- make zrpc.connection.Connection.handle_request do some fancier work
around 581 to check what parameters the method is expecting. I worry
that might impact performance though...

Help?

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Jim Fulton]

On Thu, Nov 19, 2009 at 5:38 PM, Chris Withers <chris@...> wrote:

> Chris Withers wrote:
>>
>> Jim Fulton wrote:
>>>
>>> I'm fine with change the storage server to pass the history size
>>> argument as a keyword parameter.
>>
>> Okay, but where is the Z309 equivalent of ZEOStorage308Adapter's history
>> method?
>
> Ug, the answer is much less pleasant :-(
>
> So, a ZODB 3.9.3 ClientStorage calls the ServerStub's history method, this
> does an rpc call:
>
> self.rpc.call('history', oid, length)
>
> Which comes in on the server and gets despatched straight out in:
>
> /ZEO/zrpc/connection.py", line 581, in handle_request
>
> This, of course, means we can't pull the keyword parameters trick, since
> ZEO's rpc doesn't support keyword parameters from what I saw...
>
> I'm not sure what to suggest here...

What's wrong with the original idea of having the server pass the size
as a keyword parameter? The ZEO protocol is irrelevant. (Unless it's
relevent somehow in the guts of ZEORAID, in which case, I don't care.
:)

Jim

--
Jim Fulton
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Jim Fulton wrote:
> What's wrong with the original idea of having the server pass the size
> as a keyword parameter? The ZEO protocol is irrelevant.

Okay, from the top:

>> So, a ZODB 3.9.3 ClientStorage calls the ServerStub's history method, this
>> does an rpc call:
>>
>> self.rpc.call('history', oid, length)
>>
>> Which comes in on the server and gets despatched straight out in:
>>
>> /ZEO/zrpc/connection.py", line 581, in handle_request

...which ends up calling .history(oid,length) on zeoraid on RAIDStorage
which is implemented as follows:

     def history(self, oid, version='', size=1):
         """Return a sequence of history information dictionaries."""
         assert version is ''
         return self._apply_single_storage('history', (oid, size))[0]

...because it's designed to work with ZODB 3.8 and 3.9, but doesn't
support versions.

We can't use the trick of making the Z308-->Z309 adapter pass the size
as a keyword parameter, because the call in this case is actually in
ZODB 3.9's ClientStorage, so the adapter is never used, because all
components involved are speaking Z309.

We also can't change the ClientStorage call to pass the size as a
keyword parameter because ZEO's rpc doesn't appear to support keyword
parameters.

That leaves the ideas I posted in my last reply...

cheers,

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Jim Fulton]

On Thu, Nov 19, 2009 at 5:55 PM, Chris Withers <chris@...> wrote:

> Jim Fulton wrote:
>>
>> What's wrong with the original idea of having the server pass the size
>> as a keyword parameter? The ZEO protocol is irrelevant.
>
> Okay, from the top:
>
>>> So, a ZODB 3.9.3 ClientStorage calls the ServerStub's history method,
>>> this
>>> does an rpc call:
>>>
>>> self.rpc.call('history', oid, length)
>>>
>>> Which comes in on the server and gets despatched straight out in:
>>>
>>> /ZEO/zrpc/connection.py", line 581, in handle_request
>
> ...which ends up calling .history(oid,length) on zeoraid on RAIDStorage
> which is implemented as follows:
>
>    def history(self, oid, version='', size=1):
>        """Return a sequence of history information dictionaries."""
>        assert version is ''
>        return self._apply_single_storage('history', (oid, size))[0]
>
> ...because it's designed to work with ZODB 3.8 and 3.9, but doesn't support
> versions.
>
> We can't use the trick of making the Z308-->Z309 adapter pass the size as a
> keyword parameter, because the call in this case is actually in ZODB 3.9's
> ClientStorage, so the adapter is never used, because all components involved
> are speaking Z309.

So why not add a history method to ZEOStorage that passes size as a
keyword argument?

Jim

--
Jim Fulton
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Jim Fulton wrote:
> So why not add a history method to ZEOStorage that passes size as a
> keyword argument?

I don't know what "ZEOStorage" is...

zrpc.connection.Connection appears to despatch direct to the storage,
please correct me if I'm wrong.

In this case, that's a RAIDStorage. It doesn't have any choice in what
parameters its history method gets called with, that's controlled by the
ClientStorage emitting the zrpc call...

So I'm afraid I don't really understand what you're trying to suggest...

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Jim Fulton]

On Thu, Nov 19, 2009 at 6:07 PM, Chris Withers <chris@...> wrote:
> Jim Fulton wrote:
>>
>> So why not add a history method to ZEOStorage that passes size as a
>> keyword argument?
>
> I don't know what "ZEOStorage" is...

Then we shouldn't be having this conversation.

Jim

--
Jim Fulton
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Jim Fulton wrote:
> On Thu, Nov 19, 2009 at 6:07 PM, Chris Withers <chris@...> wrote:
>> Jim Fulton wrote:
>>> So why not add a history method to ZEOStorage that passes size as a
>>> keyword argument?
>> I don't know what "ZEOStorage" is...
>
> Then we shouldn't be having this conversation.

I'm working off this traceback:

Traceback (most recent call last):
   File "ZODB3-3.9.3-py2.6-linux-i686.egg/ZEO/zrpc/connection.py", line
581, in handle_request
     ret = meth(*args)
   File "gocept.zeoraid-1.0b6-py2.6.egg/gocept/zeoraid/storage.py", line
219, in history
     assert version is ''

There is no ZEOStorage instance involved in this as far as I can tell.
What am I missing?

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Christian Theune-2]

On 11/20/2009 01:07 AM, Chris Withers wrote:
> Jim Fulton wrote:
>> On Thu, Nov 19, 2009 at 6:07 PM, Chris Withers<chris@...>  wrote:
>>> Jim Fulton wrote:
>>>> So why not add a history method to ZEOStorage that passes size as a
>>>> keyword argument?
>>> I don't know what "ZEOStorage" is...

See StorageServer.py around line 80. As the docstring says it is a:
Proxy to underlying storage for a single remote client.

>> Then we shouldn't be having this conversation.
>
> I'm working off this traceback:
>
> Traceback (most recent call last):
>     File "ZODB3-3.9.3-py2.6-linux-i686.egg/ZEO/zrpc/connection.py", line
> 581, in handle_request
>       ret = meth(*args)
>     File "gocept.zeoraid-1.0b6-py2.6.egg/gocept/zeoraid/storage.py", line
> 219, in history
>       assert version is ''
>
> There is no ZEOStorage instance involved in this as far as I can tell.
> What am I missing?

Check the "setup_delegation" method: the history method is patched
through directly so your traceback doesn't see the ZEOStorage anymore.

--
Christian Theune · ct@...
gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany
http://gocept.com · tel [phone] · fax [phone]
Zope and Plone consulting and development

_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev

[Chris Withers]

Christian Theune wrote:

>> Traceback (most recent call last):
>>     File "ZODB3-3.9.3-py2.6-linux-i686.egg/ZEO/zrpc/connection.py", line
>> 581, in handle_request
>>       ret = meth(*args)
>>     File "gocept.zeoraid-1.0b6-py2.6.egg/gocept/zeoraid/storage.py", line
>> 219, in history
>>       assert version is ''
>>
>> There is no ZEOStorage instance involved in this as far as I can tell.
>> What am I missing?
>
> Check the "setup_delegation" method: the history method is patched
> through directly so your traceback doesn't see the ZEOStorage anymore.

OK, I've committed tests and patches to deal with this issue and the
similar issue in ZEOStorage308Adapter on the 3.9 branch.

Should I merge these changes to the trunk too?

cheers,

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
            - http://www.simplistix.co.uk
_______________________________________________
For more information about ZODB, see the ZODB Wiki:
http://www.zope.org/Wikis/ZODB/

ZODB-Dev mailing list  -  ZODB-Dev@...
https://mail.zope.org/mailman/listinfo/zodb-dev