Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Query in one tab cancelled and rolled back when another tab or window on the connection starts or stops a query.
- Stop on MySQL 5.5, 5.6 or MariaDB 5.5 interrupting the next statement on the connection.
- `QUEUED` results and hidden command errors when running several Redis commands or saving Redis grid edits.
- Redis databases unlistable on a server that removes or denies `CONFIG`, such as AWS ElastiCache, which showed the server's error in place of the keyspace.
- Query timeout ignored on MySQL before 5.7.8 and MariaDB before 10.1.1.
- MySQL query run a second time, and left running on the server, after the connection timed out.
- Empty error when a parameterized MySQL query timed out.
Expand Down
36 changes: 36 additions & 0 deletions Plugins/RedisDriverPlugin/RedisDatabaseCount.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//
// RedisDatabaseCount.swift
// RedisDriverPlugin
//
// How many logical databases the sidebar lists.
// Kept apart from the driver, which imports CRedis, so the rule can be tested without the
// C library.
//

import Foundation

nonisolated enum RedisDatabaseCount {
/// What a Redis server ships with, and what the sidebar draws when the server will not say.
static let fallback = 16

/// A cluster node answers `CONFIG GET databases` with 1, and refuses `SELECT` with any other
/// index, so the tree shows the one keyspace that exists rather than fifteen that do not.
static let cluster = 1

/// Managed Redis commonly removes `CONFIG` outright rather than denying it: AWS ElastiCache
/// lists it as restricted, so the probe answers `unknown command` and `run` throws. The count
/// only decides how many `db` entries to draw, so a server that will not answer takes the
/// fallback instead of failing the whole listing.
static func resolve(on conn: any RedisCommandChannel) async -> Int {
guard conn.supportsDatabaseSelection else { return cluster }
guard let reply = try? await conn.run(["CONFIG", "GET", "databases"]),
let array = reply.arrayValue,
array.count >= 2,
let count = array[1].intValue,
count > 0
else {
return fallback
}
return count
}
}
16 changes: 3 additions & 13 deletions Plugins/RedisDriverPlugin/RedisPluginDriver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ final class RedisPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
return [PluginTableInfo(name: Self.clusterDatabaseName, type: "TABLE", rowCount: count)]
}

let databases = try await databaseCount(on: conn)
let databases = await RedisDatabaseCount.resolve(on: conn)
let result = try await conn.run(["INFO", "keyspace"])
let info = result.stringValue ?? ""

Expand All @@ -261,17 +261,6 @@ final class RedisPluginDriver: PluginDatabaseDriver, @unchecked Sendable {

static let clusterDatabaseName = "db0"

/// A cluster node answers CONFIG GET databases with 1, and refuses SELECT with any other
/// index, so the tree shows the one keyspace that exists rather than fifteen that do not.
private func databaseCount(on conn: any RedisCommandChannel) async throws -> Int {
guard conn.supportsDatabaseSelection else { return 1 }
let reply = try await conn.run(["CONFIG", "GET", "databases"])
guard let array = reply.arrayValue, array.count >= 2, let count = array[1].intValue, count > 0 else {
return 16
}
return count
}

func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] {
[
PluginColumnInfo(name: "Key", dataType: "String", isNullable: false, isPrimaryKey: true),
Expand Down Expand Up @@ -369,7 +358,8 @@ final class RedisPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
guard let conn = redisConnection else {
throw RedisPluginError.notConnected
}
return try await (0 ..< databaseCount(on: conn)).map { "db\($0)" }
let databases = await RedisDatabaseCount.resolve(on: conn)
return (0 ..< databases).map { "db\($0)" }
}

func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata {
Expand Down
103 changes: 103 additions & 0 deletions TableProTests/Plugins/RedisDatabaseCountTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
//
// RedisDatabaseCountTests.swift
// TableProTests
//
// AWS ElastiCache removes CONFIG rather than denying it, so the probe for how many databases to
// list answers `unknown command` and the run choke point throws. That throw used to escape
// `fetchTables` and `fetchDatabases`, and the sidebar showed the server's error instead of the
// keyspace, on every ElastiCache node.
//

import Foundation
import TableProPluginKit
import Testing

/// Driven by one task at a time, so the replies are handed out in order with no synchronisation.
private final class StubRedisChannel: RedisCommandChannel, @unchecked Sendable {
private var queuedReplies: [RedisReply]
private let selectable: Bool
private(set) var sentCommands: [[String]] = []

init(_ replies: [RedisReply], supportsDatabaseSelection selectable: Bool = true) {
queuedReplies = replies
self.selectable = selectable
}

var isConnected: Bool { true }
var supportsDatabaseSelection: Bool { selectable }

func connect(reportingStage report: @escaping ConnectionStageReporter) async throws {}
func disconnect() {}
func cancelCurrentQuery() {}
func serverVersion() -> String? { "8.10.1" }
func currentDatabase() -> Int { 0 }
func selectDatabase(_ index: Int) async throws {}

func executeCommand(_ args: [Data]) async throws -> RedisReply {
sentCommands.append(args.map { String(data: $0, encoding: .utf8) ?? "" })
guard !queuedReplies.isEmpty else { return .null }
return queuedReplies.removeFirst()
}

func executePipeline(_ commands: [[Data]]) async throws -> [RedisReply] {
var replies: [RedisReply] = []
for command in commands {
replies.append(try await executeCommand(command))
}
return replies
}
}

@Suite("Redis database count")
struct RedisDatabaseCountTests {
/// Measured on Redis 8.10.1 started with `--rename-command CONFIG ''`, which is what
/// ElastiCache does: the server answers with an error naming a command it does not have,
/// not with `NOPERM`.
@Test("A server that removes CONFIG still lists the default 16")
func removedConfigFallsBack() async {
let channel = StubRedisChannel([
.error("ERR unknown command 'CONFIG', with args beginning with: 'GET' 'databases'"),
])
#expect(await RedisDatabaseCount.resolve(on: channel) == 16)
}

@Test("A server that denies CONFIG through an ACL lists the default 16")
func deniedConfigFallsBack() async {
let channel = StubRedisChannel([
.error("NOPERM User viewer has no permissions to run the 'config|get' command"),
])
#expect(await RedisDatabaseCount.resolve(on: channel) == 16)
}

@Test("A server that answers is taken at its word")
func serverAnswerWins() async {
let channel = StubRedisChannel([.array([.string("databases"), .string("4")])])
#expect(await RedisDatabaseCount.resolve(on: channel) == 4)
#expect(channel.sentCommands == [["CONFIG", "GET", "databases"]])
}

static let unusableAnswers: [RedisReply] = [
.array([.string("databases")]),
.array([.string("databases"), .string("none")]),
.array([.string("databases"), .string("0")]),
.array([.string("databases"), .string("-1")]),
.array([]),
.status("OK"),
.null,
]

@Test("An answer that is not a count falls back", arguments: unusableAnswers)
func unusableAnswerFallsBack(reply: RedisReply) async {
#expect(await RedisDatabaseCount.resolve(on: StubRedisChannel([reply])) == 16)
}

/// A cluster node has one keyspace and refuses `SELECT` with any other index, so it is never
/// asked: the probe would cost a round trip to learn a number the topology already fixes.
@Test("A cluster node is one keyspace and is never probed")
func clusterIsNotProbed() async {
let channel = StubRedisChannel([.array([.string("databases"), .string("16")])],
supportsDatabaseSelection: false)
#expect(await RedisDatabaseCount.resolve(on: channel) == 1)
#expect(channel.sentCommands.isEmpty)
}
}
1 change: 1 addition & 0 deletions project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ targets:
- Plugins/RedisDriverPlugin/RedisCommandRouting.swift
- Plugins/RedisDriverPlugin/RedisConnectProbe.swift
- Plugins/RedisDriverPlugin/RedisConnectionMode.swift
- Plugins/RedisDriverPlugin/RedisDatabaseCount.swift
- Plugins/RedisDriverPlugin/RedisDatabaseIndex.swift
- Plugins/RedisDriverPlugin/RedisKeySlot.swift
- Plugins/RedisDriverPlugin/RedisKeySummary.swift
Expand Down