Support for client-side caching - phase 2 #3673
Conversation
sazzad16
requested review from
chayim and
uglide
and removed request for
uglide
chayim
changed the title
| @@ -24,7 +24,7 @@ public JedisClientSideCache(final HostAndPort hostPort, final JedisClientConfig | |||
|
|
|||
| private void clientTrackingOn() { | |||
| String reply = connection.executeCommand(new CommandObject<>( | |||
| new CommandArguments(Protocol.Command.CLIENT).add("TRACKING").add("ON").add("BCAST"), | |||
| new CommandArguments(Protocol.Command.CLIENT).add("TRACKING").add("ON"), | |||
There was a problem hiding this comment.
Correct, though make sure you're cognizant of (hopefully) future support for broadcast and friends.
| ensureFillSafe(); | ||
| return buf[count]; | ||
| public boolean peek(byte b) throws JedisConnectionException { | ||
| ensureFill(); // in current design, at least one reply is expected. so ensureFillSafe() is not necessary. |
There was a problem hiding this comment.
But, what happens if there is no reply? It shouldn't occur, but maybe there's something we can do for resiliency here. At the very least raise a data specific exception so that we can know this happened?
There was a problem hiding this comment.
@chayim A JedisConnectionException wrapping a ReadTimeoutException will be thrown.
| jedis.set("foo", "bar"); | ||
| assertEquals("bar", jCache.get("foo")); | ||
| jedis.del("foo"); | ||
| assertNull(jCache.get("foo")); | ||
| assertThat(jCache.get("foo"), Matchers.oneOf("bar", null)); // ? |
There was a problem hiding this comment.
Can this be achieved without adding this import?
There was a problem hiding this comment.
@chayim We'd have to write a "utility" method.
But why bother? We already have these as our test dependency and being used in some other tests.
There was a problem hiding this comment.
General desire to have fewer dependencies.. even if it's just for test. Not a hard requirement.
No description provided.