Darrel Schneider created GEODE-9513:
---------------------------------------
Summary: optimize redis SCAN commands cursor type
Key: GEODE-9513
URL: https://issues.apache.org/jira/browse/GEODE-9513
Project: Geode
Issue Type: Improvement
Components: redis
Reporter: Darrel Schneider
The geode SCAN commands implement the cursor with a BigInteger type. It seems
like it could instead be a "long". The following are some discussions on slack
about this:
Darrel Schneider:
Does anyone know why the redis HSCAN, SSCAN, and SCAN cursor in our
implementation is a BigInteger? This seems like overkill. I would think in our
implementation a 64-bit signed value would be big enough for the scan cursor.
Donal Evans
I think it's to ensure that our input values match native Redis. Redis takes
values in the range of BigInteger for cursor, even though they have the same
constraints on the number of elements in their collections that we do
Darrel Schneider
The redis docs say:
_Calling SCAN with a broken, negative, out of range, or otherwise invalid
cursor, will result into undefined behavior but never into a crash. What will
be undefined is that the guarantees about the returned elements can no longer
be ensured by the SCAN implementation.
The only valid cursors to use are:
The cursor value of 0 when starting an iteration.
The cursor returned by the previous call to SCAN in order to continue the
iteration.
_
Since we will never return a cursor larger than a signed 64-bit number is seems
like if we see one come in we could immediately return an empty scan since that
is what we will do anyway in our current implementation but we do a bunch of
work to figure that out. Since we never return a cursor larger than
Long.MAX_VALUE it seems like we could do some validation on the input and only
if it looks like a valid cursor do a real scan. That would allow our scan impl
to use a "long" instead of a BigInteger. Since BigInteger is immutable
incrementing it produces a bunch of garbage. So it seems like we could be
compatible with native redis without using a BigInteger all the time. Let me
know if I'm missing something. (edited)
New
Donal Evans
I think that's a good idea, but we'd probably still have to create a BigInteger
at least once, to cover all the bases. There are three cases we have to deal
with here, I think: 1. the cursor is a valid positive Long, in which case we
parse it and use it. 2. the cursor is a positive integer larger than
Long.MAX_VALUE but smaller than the capacity of an unsigned long (which is the
largest value Redis accepts) in which case we just return an empty scan 3. the
cursor is negative, larger than the capacity of an unsigned long or not an
integer, in which case we return an error. Telling the difference between cases
2 and 3 is where we might run into trouble, since we've been trying to exactly
mirror Redis' behaviour when it comes to errors
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
