Skip to content

Commit

Permalink
Add Valkey8 support
Browse files Browse the repository at this point in the history
Signed-off-by: Shoham Elias <shohame@amazon.com>
  • Loading branch information
shohamazon committed Aug 20, 2024
1 parent 048812b commit 50751c8
Show file tree
Hide file tree
Showing 13 changed files with 292 additions and 106 deletions.
4 changes: 4 additions & 0 deletions .github/json_matrices/engine-matrix.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,9 @@
{
"type": "valkey",
"version": "7.2.5"
},
{
"type": "valkey",
"version": "8.0.0-rc1"
}
]
1 change: 1 addition & 0 deletions .github/workflows/install-valkey/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ runs:
echo 'export PATH=/usr/local/bin:$PATH' >>~/.bash_profile
- name: Verify Valkey installation and symlinks
if: ${{ !contains(inputs.engine-version, '-rc1') }}
shell: bash
run: |
# In Valkey releases, the engine is built with symlinks from valkey-server and valkey-cli
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ on:
- .github/workflows/lint-rust/action.yml
- .github/workflows/install-valkey/action.yml
- .github/json_matrices/build-matrix.json
- .github/json_matrices/engine-matrix.json

pull_request:
paths:
Expand All @@ -29,6 +30,7 @@ on:
- .github/workflows/lint-rust/action.yml
- .github/workflows/install-valkey/action.yml
- .github/json_matrices/build-matrix.json
- .github/json_matrices/engine-matrix.json
workflow_dispatch:

concurrency:
Expand Down
51 changes: 46 additions & 5 deletions java/integTest/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,53 @@ tasks.register('startStandalone') {

tasks.register('getServerVersion') {
doLast {
new ByteArrayOutputStream().withStream { os ->
exec {
commandLine 'redis-server', '-v'
standardOutput = os
def detectedVersion
def output = new ByteArrayOutputStream()

// Helper method to find the full path of a command
def findFullPath = { command ->
def pathOutput = new ByteArrayOutputStream()
try {
exec {
commandLine 'which', command // Use 'where' for Windows
standardOutput = pathOutput
}
return pathOutput.toString().trim()
} catch (Exception e) {
println "Failed to find path for ${command}: ${e.message}"
return ""
}
}

// Get full paths
def valkeyPath = findFullPath('valkey-server')
def redisPath = findFullPath('redis-server')

def tryGetVersion = { serverPath ->
try {
exec {
commandLine serverPath, '-v'
standardOutput = output
}
return output.toString()
} catch (Exception e) {
println "Failed to execute ${serverPath}: ${e.message}"
return ""
}
serverVersion = extractServerVersion(os.toString())
}

// Try valkey-server first, then redis-server if it fails
def versionOutput = tryGetVersion(valkeyPath)
if (versionOutput.isEmpty() && !redisPath.isEmpty()) {
versionOutput = tryGetVersion(redisPath)
}

if (!versionOutput.isEmpty()) {
detectedVersion = extractServerVersion(versionOutput)
println "Detected server version: ${detectedVersion}"
serverVersion = detectedVersion
} else {
throw new GradleException("Failed to retrieve the server version.")
}
}
}
Expand Down
66 changes: 48 additions & 18 deletions java/integTest/src/test/java/glide/SharedCommandTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -13774,9 +13774,14 @@ public void sscan(BaseClient client) {
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);

// Negative cursor
result = client.sscan(key1, "-1").get();
assertEquals(initialCursor, result[resultCursorIndex]);
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> client.sscan(key1, "-1").get());
} else {
result = client.sscan(key1, "-1").get();
assertEquals(initialCursor, result[resultCursorIndex]);
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
}

// Result contains the whole set
assertEquals(charMembers.length, client.sadd(key1, charMembers).get());
Expand Down Expand Up @@ -13910,9 +13915,14 @@ public void sscan_binary(BaseClient client) {
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);

// Negative cursor
result = client.sscan(key1, gs("-1")).get();
assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> client.sscan(key1, gs("-1")).get());
} else {
result = client.sscan(key1, gs("-1")).get();
assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
}

// Result contains the whole set
assertEquals(charMembers.length, client.sadd(key1, charMembers).get());
Expand Down Expand Up @@ -14059,9 +14069,14 @@ public void zscan(BaseClient client) {
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);

// Negative cursor
result = client.zscan(key1, "-1").get();
assertEquals(initialCursor, result[resultCursorIndex]);
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> client.zscan(key1, "-1").get());
} else {
result = client.zscan(key1, "-1").get();
assertEquals(initialCursor, result[resultCursorIndex]);
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
}

// Result contains the whole set
assertEquals(charMembers.length, client.zadd(key1, charMap).get());
Expand Down Expand Up @@ -14240,9 +14255,14 @@ public void zscan_binary(BaseClient client) {
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);

// Negative cursor
result = client.zscan(key1, gs("-1")).get();
assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> client.zscan(key1, gs("-1")).get());
} else {
result = client.zscan(key1, gs("-1")).get();
assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
}

// Result contains the whole set
assertEquals(charMembers.length, client.zadd(key1.toString(), charMap_strings).get());
Expand Down Expand Up @@ -14425,9 +14445,14 @@ public void hscan(BaseClient client) {
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);

// Negative cursor
result = client.hscan(key1, "-1").get();
assertEquals(initialCursor, result[resultCursorIndex]);
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> client.hscan(key1, "-1").get());
} else {
result = client.hscan(key1, "-1").get();
assertEquals(initialCursor, result[resultCursorIndex]);
assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
}

// Result contains the whole set
assertEquals(charMembers.length, client.hset(key1, charMap).get());
Expand Down Expand Up @@ -14589,9 +14614,14 @@ public void hscan_binary(BaseClient client) {
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);

// Negative cursor
result = client.hscan(key1, gs("-1")).get();
assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> client.hscan(key1, gs("-1")).get());
} else {
result = client.hscan(key1, gs("-1")).get();
assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
}

// Result contains the whole set
assertEquals(charMembers.length, client.hset(key1, charMap).get());
Expand Down
28 changes: 19 additions & 9 deletions java/integTest/src/test/java/glide/cluster/CommandTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -1051,15 +1051,19 @@ public void flushall() {
assertEquals(OK, clusterClient.flushall(ASYNC, route).get());

var replicaRoute = new SlotKeyRoute("key", REPLICA);
// command should fail on a replica, because it is read-only
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> clusterClient.flushall(replicaRoute).get());
assertInstanceOf(RequestException.class, executionException.getCause());
assertTrue(
executionException
.getMessage()
.toLowerCase()
.contains("can't write against a read only replica"));
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
assertEquals(OK, clusterClient.flushall(route).get());
} else {
// command should fail on a replica, because it is read-only
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> clusterClient.flushall(replicaRoute).get());
assertInstanceOf(RequestException.class, executionException.getCause());
assertTrue(
executionException
.getMessage()
.toLowerCase()
.contains("can't write against a read only replica"));
}
}

// TODO: add a binary version of this test
Expand Down Expand Up @@ -1640,6 +1644,9 @@ public void fcall_binary_with_keys(String prefix) {
@Test
public void fcall_readonly_function() {
assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in version 7");
assumeTrue(
!SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0"),
"Temporary disabeling this test on valkey 8");

String libName = "fcall_readonly_function";
// intentionally using a REPLICA route
Expand Down Expand Up @@ -1695,6 +1702,9 @@ public void fcall_readonly_function() {
@Test
public void fcall_readonly_binary_function() {
assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in version 7");
assumeTrue(
!SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0"),
"Temporary disabeling this test on valkey 8");

String libName = "fcall_readonly_function";
// intentionally using a REPLICA route
Expand Down
48 changes: 36 additions & 12 deletions java/integTest/src/test/java/glide/standalone/CommandTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -1535,9 +1535,14 @@ public void scan() {
assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);

// Negative cursor
Object[] negativeResult = regularClient.scan("-1").get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> regularClient.scan("-1").get());
} else {
Object[] negativeResult = regularClient.scan("-1").get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
}

// Add keys to the database using mset
regularClient.mset(keys).get();
Expand Down Expand Up @@ -1589,9 +1594,14 @@ public void scan_binary() {
assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);

// Negative cursor
Object[] negativeResult = regularClient.scan(gs("-1")).get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
ExecutionException executionException =
assertThrows(ExecutionException.class, () -> regularClient.scan(gs("-1")).get());
} else {
Object[] negativeResult = regularClient.scan(gs("-1")).get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
}

// Add keys to the database using mset
regularClient.msetBinary(keys).get();
Expand Down Expand Up @@ -1652,9 +1662,16 @@ public void scan_with_options() {
assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);

// Negative cursor
Object[] negativeResult = regularClient.scan("-1", options).get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
final ScanOptions finalOptions = options;
ExecutionException executionException =
assertThrows(
ExecutionException.class, () -> regularClient.scan("-1", finalOptions).get());
} else {
Object[] negativeResult = regularClient.scan("-1", options).get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
}

// scan for strings by match pattern:
options =
Expand Down Expand Up @@ -1734,9 +1751,16 @@ public void scan_binary_with_options() {
assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);

// Negative cursor
Object[] negativeResult = regularClient.scan(gs("-1"), options).get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
final ScanOptions finalOptions = options;
ExecutionException executionException =
assertThrows(
ExecutionException.class, () -> regularClient.scan(gs("-1"), finalOptions).get());
} else {
Object[] negativeResult = regularClient.scan(gs("-1"), options).get();
assertEquals(initialCursor, negativeResult[resultCursorIndex]);
assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
}

// scan for strings by match pattern:
options =
Expand Down
42 changes: 31 additions & 11 deletions node/tests/SharedTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,7 @@ export function runBaseTests<Context>(config: {
it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
`hscan and sscan empty set, negative cursor, negative count, and non-hash key exception tests`,
async (protocol) => {
await runTest(async (client: BaseClient) => {
await runTest(async (client: BaseClient, cluster: RedisCluster) => {
const key1 = "{key}-1" + uuidv4();
const key2 = "{key}-2" + uuidv4();
const initialCursor = "0";
Expand All @@ -1481,13 +1481,23 @@ export function runBaseTests<Context>(config: {
expect(result[resultCollectionIndex]).toEqual([]);

// Negative cursor
result = await client.hscan(key1, "-1");
expect(result[resultCursorIndex]).toEqual(initialCursor);
expect(result[resultCollectionIndex]).toEqual([]);
if (cluster.checkIfServerVersionLessThan("7.9.0")) {
result = await client.hscan(key1, "-1");
expect(result[resultCursorIndex]).toEqual(initialCursor);
expect(result[resultCollectionIndex]).toEqual([]);

result = await client.sscan(key1, "-1");
expect(result[resultCursorIndex]).toEqual(initialCursor);
expect(result[resultCollectionIndex]).toEqual([]);
} else {
await expect(client.hscan(key1, "-1")).rejects.toThrow(
RequestError,
);

result = await client.sscan(key1, "-1");
expect(result[resultCursorIndex]).toEqual(initialCursor);
expect(result[resultCollectionIndex]).toEqual([]);
await expect(client.sscan(key1, "-1")).rejects.toThrow(
RequestError,
);
}

// Exceptions
// Non-hash key
Expand Down Expand Up @@ -7525,7 +7535,7 @@ export function runBaseTests<Context>(config: {
it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
`zscan test_%p`,
async (protocol) => {
await runTest(async (client: BaseClient) => {
await runTest(async (client: BaseClient, cluster: RedisCluster) => {
const key1 = "{key}-1" + uuidv4();
const key2 = "{key}-2" + uuidv4();
const initialCursor = "0";
Expand Down Expand Up @@ -7556,9 +7566,19 @@ export function runBaseTests<Context>(config: {
expect(result[resultCollectionIndex]).toEqual([]);

// Negative cursor
result = await client.zscan(key1, "-1");
expect(result[resultCursorIndex]).toEqual(initialCursor);
expect(result[resultCollectionIndex]).toEqual([]);
if (cluster.checkIfServerVersionLessThan("7.9.0")) {
result = await client.zscan(key1, "-1");
expect(result[resultCursorIndex]).toEqual(initialCursor);
expect(result[resultCollectionIndex]).toEqual([]);
} else {
try {
expect(await client.zscan(key1, "-1")).toThrow();
} catch (e) {
expect((e as Error).message).toMatch(
"ResponseError: invalid cursor",
);
}
}

// Result contains the whole set
expect(await client.zadd(key1, charMap)).toEqual(
Expand Down
Loading

0 comments on commit 50751c8

Please sign in to comment.