Bug 745832 (EDG-76) - cas unique value should be treated as 64bit unsigned integer
Summary: cas unique value should be treated as 64bit unsigned integer
Keywords:
Status: CLOSED NOTABUG
Alias: EDG-76
Product: JBoss Data Grid 5
Classification: JBoss
Component: Infinispan
Version: unspecified
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ---
: EAP 5.1.0 EDG TP
Assignee: Default User
QA Contact:
URL: http://jira.jboss.org/jira/browse/EDG-76
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-11-22 14:47 UTC by Michal Linhard
Modified: 2014-03-17 04:01 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-11-25 18:10:06 UTC
Type: Bug


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker EDG-76 0 None None None Never

Description Michal Linhard 2010-11-22 14:47:05 UTC
project_key: EDG

in memcached server module 
we should allow values 0 .. 18446744073709551615 for <cas unique> argument in the command 
cas <key> <flags> <exptime> <bytes> <cas unique> [noreply]\r\n

testcase:
$ echo -e "cas a 0 0 1 18446744073709551615 \r\na\r" | nc localhost 11211
SERVER_ERROR org.infinispan.server.core.ServerException: java.lang.NumberFormatException: For input string: "18446744073709551615"

Comment 1 Galder Zamarreño 2010-11-24 15:08:36 UTC
Hmmmm, the document says nothing about cas unique being an unsigned integer. All it says is:

"<cas unique> is a unique 64-bit value of an existing entry."
"<cas unique> is a unique 64-bit integer"

There's no reference o unsigned actually, so a simple Java long should do.

Comment 2 Galder Zamarreño 2010-11-24 15:10:43 UTC
Remember as well that the cas unique is not necessarily a counter. It can be implemented as such but the aim is a different one. It tries to identify modifications in a unique way. In fact, we've implemented it so that part of it is a counter but the other might not, particularly if running in a cluster.

Comment 3 Michal Linhard 2010-11-24 15:59:06 UTC
Yep you're right there's no mention of "unsigned"
I just got carried away, because the original implementation allows these values (0 .. 18446744073709551615) which means that they are parsing it as unsigned integer.

The way casid is used (for optimistic locking) is:
1. set
2. gets, store returned casid
3. cas, pass casid as parameter

so in cas command the server never gets a wrong value, because he generated it himself.
problem can be on client side in step2 if client only expects unsigned 64bit integer.
can we generate negative casid ?
the way it is implemented now: <view id (2 bytes)><rank (2 bytes)><version counter (4 bytes)>
can view id sometimes have 1 as MSB ? if this never happens we're safe...
if yes we could maybe switch view_id and rank order because rank's basically number of nodes in view which hardly even exceeds 1000.



Comment 4 Galder Zamarreño 2010-11-25 10:36:56 UTC
I agree that rank reaching 1000 is less unlikely than view ID, which are the number of view changes.

It's quite an edge case tbh, but as you said, I'd swap around the rank and view ids to protect against this just in case.

I need to verify how easy is it to keep this value within the unsigned ranges. I don't want add too much complexity here cos it's a part of the code that is used a lot. Each modification generates a new cas/version, so I'd rather keep it efficient than dealing with negative values here.

Comment 5 Galder Zamarreño 2010-11-25 10:55:46 UTC
Link: Added: This issue depends ISPN-802


Comment 6 Galder Zamarreño 2010-11-25 18:09:24 UTC
I'm rejecting this since the Memcached spec is not specific enough in terms of whether the cas/version number should be unsigned or not.

Comment 7 Galder Zamarreño 2010-11-25 18:10:06 UTC
I'm rejecting this since the Memcached spec is not specific enough in terms of whether the cas/version number should be unsigned or not.


Note You need to log in before you can comment on or make changes to this bug.