Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Valkey8 support #2102

Closed
wants to merge 1 commit into from
+292 −106
Closed

Add Valkey8 support #2102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/json_matrices/engine-matrix.json
View file
Open in desktop
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
View file
Open in desktop
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') }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's too specific. change to

Suggested change
if: ${{ !contains(inputs.engine-version, '-rc1') }}
if: ${{ !contains(inputs.engine-version, '-rc') }}

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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")) {
shohamazon marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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")) {
shohamazon marked this conversation as resolved.
Show resolved Hide resolved
assertEquals(OK, clusterClient.flushall(route).get());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since valkey 8.0.8 flushall can run on replicas

} 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
View file
Open in desktop
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
View file
Open in desktop
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
Loading