- 02 Dec, 2020 1 commit
-
-
Nathan Harris authored
Motivation: RediStack today represents a command as a temporary object for the purpose of writing to the channel. While it is useful to have an object for that purpose, commands handled in this way require immediate execution and aren't available for other purposes. Commands can serve a better purpose as a lightweight object to support delayed execution, so that pipeling as described in issue #63 could be possible. Modifications: - Add: `get` overloads for JSON codable interaction on `RedisClient` - Add: New `RedisZRangeResultOption` type for better interactions with zrange operations that can optionally return scores - Add: New `RedisHashFieldKey` for type-safety when working with Hash field keys much like `RedisKey` - Change: A few API types from enums to structs for library evolution - Change: `RedisCommandHandler` to operate on a tuple of `RESPValue, EventLoopPromise<RESPValue>` rather than `RedisCommand` - Change: `RedisCommand` to be a generic struct with the keyword, arguments, and a transform closure to defer execution - Change: Almost all `RedisClient` command extensions to be factory methods on `RedisCommand` instead - Change: Many response types to be optional to avoid developers having to do `isNull` checks on their own - Change: `RedisClient.send(command:arguments:)` to be generic with `send(_:)` as the signature - Rename: RedisClient extensions for scan methods to be more discoverable and legible as `scanHashField`, etc. Result: It should be easier to support a clean pipelining API with deferred command execution, with extensions being easier for 2nd party developers, and the maintenance overhead of all of the command extensions should be a little lighter when it comes to changing HOW commands are sent such as adding a context parameter
-
- 27 Nov, 2020 1 commit
-
-
Nathan Harris authored
Motivation: `RedisKeyLifetime` already has "RedisKey" as a prefix so it naturally fits as a nested type. Modifications: - Change: `RedisKeyLifetime` to be nested in `RedisKey` and named `Lifetime` - Rename: `RedisKeyLifetime.Lifetime` to `Duration` - Deprecate: `RedisKeyLifetime` and the nested type `Lifetime` Result: The global namespace is a little less cluttered with the types falling naturally where they already are.
-
- 22 Nov, 2020 1 commit
-
-
Nathan Harris authored
-
- 18 Oct, 2020 1 commit
-
-
Nathan Harris authored
-
- 17 Oct, 2020 2 commits
-
-
Nathan Harris authored
Motivation: When `RedisConnection.allowSubscriptions` is set to `false`, the connection could still be in a subscription state leaving further commands to fail slowly from a full roundtrip to Redis, rather than succeeding as expected. This changes the implementation so that it triggers a full unsubscribe from patterns and channels when set to `false`. Modifications: - Change: `RedisConnection.allowSubscriptions` to call `unsubscribe()` and `punsubscribe()` when set to `false` - Change: `RedisPubSubHandler` to prefix storage of all dictionary keys to avoid name clashes between pattern and channel subscriptions Result: Developers should now have more deterministic and unsurprising behavior with PubSub in regards to subscription management and connection state.
-
Nathan Harris authored
Motivation: The methods of unsubscribing from all channels / patterns were not working as expected as they need to be special-case handled. Modifications: - Change: `RedisPubSubHandler` to be special-case unsubscribe when no arguments are provided Result: Developers should now properly be able to unsubscribe from all channels / patterns with a single method call.
-
- 15 Oct, 2020 1 commit
-
-
Nathan Harris authored
Motivation: Some Redis commands are very connection specific that have impacts on future access that makes it difficult in the current checkout-use-return cycle that `RedisConnectionPool` uses. Developers need a way to borrow a specific connection, chain several commands together, and then return the connection to the pool. Modifications: - Add: `leaseConnection` method to `RedisConnectionPool` which provides a connection from the pool and returns it after a provided closure's ELF resolves - Add: `allowSubscriptions` property to `RedisConnection` for controlling the ability to make PubSub subscriptions - Add: `RedisClientError.pubsubNotAllowed` case for when `RedisConnection.allowSubscriptions` is set to `false` and a subscription was still attempted Result: Developers should now have an "escape hatch" with `RedisConnectionPool` to do limited exclusive chains of operations on a specific connection.
-
- 07 Oct, 2020 2 commits
-
-
Nathan Harris authored
Motivation: When trying to allow users to configure the connection retry timeout offset, not having a value provided (deadline of `now`) caused all attempts to use the pool to fail. Modifications: - Change: RedisConnectionPool to always have a timeout offset defined Result: If users don't specify any value, then the default of 60 seconds will be used. If users specify "nil" (or `.none`) as the timeout, then a minimum of 10 milliseconds will be used to avoid immediate timeouts Otherwise, use the user's specified `TimeAmount` as the offset of the timeout
-
Nathan Harris authored
Motivation: With RedisConnectionPool a timeout is provided to prevent infinite loops of retrying connections, but right now it is hardcoded to 60 seconds. Users of downstream projects such as Vapor are noticing a "regression" of sorts, as previously EventLoopFutures would fail immediately if a connection was not made available. Modifications: - Add: `connectionRetryTimeout` parameter to `RedisConnectionPool` initializer that still defaults to 60 seconds - Change: RedisConnectionPool to use the new parameter if available to offset a deadline from "now" Result: Users can now configure the connection pool to fail immediately if connections are not available.
-
- 02 Oct, 2020 1 commit
-
-
Nathan Harris authored
Motivation: To ship PubSub faster, it was merged to the master branch without a peer review. This commit is to address the critical points of feedback given in a post-commit review. Modifications: - Add: New RedisClientError case where a "race condition" of removing a pubsub handler and subscription can happen - Add: New state to RedisPubSubHandler for when it has been removed from a ChannelPipeline - Change: RedisPubSubHandler to require an `eventLoop` in its initializer - Change: The subscribe and unsubscribe methods on RedisPubSubHandler to handle the EventLoop hopping to be thread-safe Result: PubSub should have a more robust and thread-safe implementation.
-
- 30 Sep, 2020 1 commit
-
-
Nathan Harris authored
Motivation: One of the great features of Redis is being able to subscribe and receive messages published to specific channels as a way of acting as a message queue for processing jobs. PubSub requires a specific understanding of the connection model that can only be implemented directly in this library. Modifications: - Add: `RedisPubSubHandler` to sit in front of `RedisCommandHandler` to manage subscription callbacks and Redis registration - Add: `publish` and the `pubsub` commands - Add: `addPubSubHandler` extension to `NIO.Channel` - Add: Type-safe String wrapper of `RedisChannelName` for PubSub methods - Add: `pubsubSubscriptionNotFound` error case - Add: `isSubscribed` property to `RedisConnection` - Add: `availableConnectionCount` and `leasedConnectionCount` properties to `RedisConnectionPool` - Add: Metrics for PubSub - Add: `makeNewPool` factory method to `RedisConnectionPoolIntegrationTestCase` - Change: `RedisClient` to require methods for PubSub management, as they are intrinsicly tied to the client's connection model - Change: Parsing of `PING` response for handling special case in PubSub mode - Rename: `ActiveConnectionGauge` to `RedisMetrics.IncrementalGauge` Result: Developers will now be able to use Redis in PubSub mode with both connections and pools. This resolves #6
-
- 08 Sep, 2020 1 commit
-
-
Nathan Harris authored
-
- 01 Sep, 2020 1 commit
-
-
Nathan Harris authored
Motivation: The original implementation of Logging was done in more haste than should have been, without proper attention given to the semantic requirements. As the Swift ecosystem has matured a bit, lessons have been learned on handling metadata and passing of external context into internal subcomponents. A mixture of the "protocol-based context passing" and "explicit context passing" patterns have been adopted. Both patterns are more fully described in the Swift forum discussion: https://forums.swift.org/t/the-context-passing-problem/39162 Modifications: - Add: `RedisLogging` namespace with references to static keys and labels that are used for Logging throughout the library - Add: `Logger` static computed properties to access the Logger prototypes used in connection and connection pools - Add: `RedisClientWithUserContext` protocol and `UserContextRedisClient` types to assist with wrapping client types for custom logger contexts - Remove: `logger` property from `RedisClient` requirements - Change: Many log statements to have higher or lower log levels for their appropriate context - Change: `RedisConnection` and `RedisConnectionPool` to conform to `RedisClientWithUserContext` - Change: `logging(to:)` protocol requirement to return a `RedisClient` existential - Change: ConnectionPool to explicitly pass a logger instance around for pooling methods Result: Logging in RediStack will now have a stronger contract of where and how logs will be generated and which context will be used. Fixes #79 and #74
-
- 05 Aug, 2020 1 commit
-
-
tanner authored
Motivation: The actual process for closing a connection pool involves closing each individual connection, which is all asynchronous. There is currently no way to work off of the event when all of the connections have completed their close process. Modifications: - Add: `poolHasActiveConnections` error value to `RedisConnectionPoolError` - Add: `promise` parameter to the `RedisConnectionPool.close` method - Add: Documentation comments for the close method Result: Developers should now have a way of chaining callbacks when all connections in a pool have been closed.
-
- 02 Aug, 2020 1 commit
-
-
Nathan Harris authored
-
- 21 Jun, 2020 1 commit
-
-
Nathan Harris authored
Motivation: As originally pointed out in #48, the `map` prefix alone is not enough context into what the method actually does. Since it fails the future, and isn't a mapping function, the name should reflect this. But, this method as-is currently provides little value outside of the client context, so it should not be `public`. If users provide an adequate use case for having it outside of the package, then it can be made public again. Modifications: - Rename: ELF where Value == RESPValue extension `map` to `tryConverting` - Change: `tryConverting` from public to internal Result: A "problematic" method should no longer be available for API users to hurt themselves with.
-
- 04 Jun, 2020 4 commits
-
-
Nathan Harris authored
Motivation: Much like the change for `RedisClientError` (6471a2) any error enum that we may want to change will necessitate a major SemVer. To avoid this, we need to use the established struct-as-enum pattern. Modifications: - Change: `RESPTranslator.ParsingError` to be a struct backed by an enum Result: The library should be able to evolve to add more `RESPTranslator.ParsingError` cases without becoming a breaking change.
-
George Barnett authored
Motivation: The SETEX and PSETEX commands are missing. Modifications: - Add SETEX command - Add PSETEX command - Add integration tests Result: Users can atomically set a key with an expire
-
George Barnett authored
Motivation: SET has a range of options for setting expirations and conditionally setting a value. Modification: - Add another `set` function with a range of options. Options are modelled as `struct`s backed by private `enum`s to allow additional options to be added without breaking API. - Added tests Result: Options may be specified with `set`, and resolves #67
-
George Barnett authored
Motivation: - Newer compilers warn that "'exported: true' has no effect in '_specialize' attribute" - Specialize is underscored so (probably) shouldn't be relied upon outside of the stdlib Modifications: - Replace @_specialize with @inlinable Result: Fewer warnings on more recent compilers
-
- 03 Jun, 2020 2 commits
-
-
Lukasa authored
Motivation: Users of Redis will frequently want to be able to run queries in parallel, while bounding the number of connections they use. They will also often want to be able to reuse connections, without having to arrange to manage those connections themselves. These are jobs usually done by a Connection Pool. This new connection pool will conform to `RedisClient` so a pool of clients and a single connection are interchangeable. Connection Pools come in a wide range of shapes and sizes. In NIO applications and frameworks, there are a number of questions that have to be answered by any pool implementation: 1. Is the pool safe to share across EventLoops: that is, is its interface thread-safe? 2. Is the pool _tied_ to an EventLoop: that is, can the pool return connections that belong on lots of event loops, or just one? 3. If the pool is not tied to an EventLoop, is it possible to influence its choice about what event loop it uses for a given connection? Question 1 is straightforward: it is almost always a trivial win to ensure that the public interface to a connection pool is thread-safe. NIO makes it possible to do this fairly cheaply in the case when the pool is only used on a single loop. Question 2 is a lot harder. Pools that are not tied to a specific EventLoop have two advantages. The first is that it is easier to bound maximum concurrency by simply configuring the pool, instead of needing to do math on the number of pools and the number of event loops. The second is that non-tied pools can arrange to keep busy applications close to this maximum concurrency regardless of how the application spreads its load across loops. However, pools that are tied to a specific EventLoop have advantages too. The first is one of implementation simplicity. As they always serve connections on a single EventLoop, they can arrange to have all of their state on that event loop too. This avoids the need to acquire locks on that loop, making internal state management easier and more obviously correct without having to worry about how long locks are held for. The second advantage is that they can be used for latency sensitive use-cases without needing to go to the work of (3). In cases where latency is very important, it can be valuable to ensure that any Channel that needs a connection can get one on the same event loop as itself. This avoids the need to thread-hop in order to communicate between the pooled connection and the user connection, reducing the latency of operations. Given the simplicity and latency benefits (which we deem particularly important for Redis use-cases), we concluded that a good initial implementation will be a pool that has a thread-safe interface, but is tied to a single EventLoop. This allows a compact, easy-to-verify implementation of the pool with great low-latency performance and simple implementation logic, that can still be accessed from any EventLoop in cases when latency is not a concern. Modifications: - Add new internal `ConnectionPool` object - Add new `RedisConnectionPool` object - Add new `RedisConnectionPoolError` type - Add tests for new types Results: Users will have access to a pooled Redis client.
-
George Barnett authored
Motivation: `RedisKey` is `ExpressibleByStringLiteral` but not `ExpressibleByStringInterpolation` which is often just as useful. Modifications: - Add `ExpressibleByStringInterpolation` conformance to `RedisKey` Result: Users can create `RedisKey`s using string interpolation
-
- 02 Jun, 2020 1 commit
-
-
George Barnett authored
Motivation: The SETNX command is missing. Modifications: - Add SETNX command - Add integration test Result: Users can set a key only if it does not already exist
-
- 01 Jun, 2020 1 commit
-
-
Nathan Harris authored
Motivation: The current state of Swift does not leave room for library evolution of enum types used for `Error`. To avoid having to increment Major SemVer to add a new error case that might be needed to fix a bug, the `enum-like struct` idiom should be used. Ideally this idiom will disappear, when Swift provides a way for Swift Packages to have a "library evolution" capability. See https://forums.swift.org/t/extensible-enumerations-for-non-resilient-libraries/35900 Modifications: - Change: `RedisClientError` to be struct with private enum value Result: Should new error cases be necessary to add, they can be in Minor SemVer releases, rather than Major SemVer.
-
- 29 May, 2020 2 commits
-
-
George Barnett authored
Motivation: The TTL and PTTL commands are missing. Modifications: - Add TTL and PTTL commands - Add integration tests Result: - Users can query the ttl in seconds or milliseconds of a key
-
George Barnett authored
Motivation: The EXISTS command was missing. Modifications: - Add 'EXISTS' to basic commands - Add integration tests Result: The existence of a key can be checked.
-
- 27 May, 2020 2 commits
-
-
Lukasa authored
Motivation: parseInteger did not distinguish between not having enough bytes for an integer and not being able to parse the integer that was present. This was a bit tricky for code internally, where some call sites had extra code looking for spooky action at a distance in order to determine if the integer failed to parse. This is unnecessary: parseInteger is sufficiently aware of what's going on to address this problem itself. Modifications: - Added a new parser error (acceptable as we haven't tagged 1.0 yet). - Throw it from parseInteger if the integer is invalid. Result: parseInteger clearly communicates if the integer failed to parse.
-
Lukasa authored
Motivation: ByteBufferView is not zero indexed, but parseSimpleString assumes it is. Modifications: - Correctly compute on the distance between two indices. - New, somewhat contrived, test case. Result: No functional change: because RediStack assumes the remote peer will always correctly terminate with /r/n, there is no point at which this code could misbehave in the current implementation. However, with small changes it is possible to trigger it, as the new test demonstrates.
-
- 26 May, 2020 2 commits
-
-
Lukasa authored
Motivation: When we only want the first byte, rather than create temporary intermediate arrays we can just ask NIO to give us the first byte. This avoids unnecessary allocations. Modifications: - Replace `readBytes(length: 1).first` with `readInteger(as: UInt8.self)` Results: 11% performance improvement in load testing due to reduced allocator pressure on the hot path.
-
Lukasa authored
Motivation: When attempting to locate a single byte, creating a transient ByteBufferView is an excessively heavyweight operation compared to a simple getInteger. In particular, a BBV requires retain/release operations to enforce the CoW invariants, as well as requires jumps through substantial amounts of generic Collection code. While this can be specialized, so can getInteger, and getInteger has much less code in the way to cause costs. Modifications: - Replace temporary view creation with getInteger. Results: 5% performance improvement on raw throughput tests.
-
- 20 Apr, 2020 1 commit
-
-
Nathan Harris authored
-
- 20 Mar, 2020 2 commits
-
-
Nathan Harris authored
Motivation: `RESPValue` exposes a fair amount of complexity on how to intialize a single instance with the various overloads. This aims to simplify the complexity for developers by providing a single initializer and relying on `RESPValueConvertible` to handle the complexities. In addition, the Swift complier synthesizes a lot of default conformances that `RedisKey` has manually written, which is just unnecessary code. Modifications: - Rename: `RESPValue.init(_:)` to `RESPValue.init(from:)` - Change: `RESPValue.init` `String?` and `FixedWidthInteger` overloads from `public` to `internal` - Remove: Unnecessary code for various protocol conformances for `RedisKey` Result: Developers should have a direct and guided way of making instances of `RESPValue`
-
Nathan Harris authored
Motivation: It was noticed that many of the commands are cumbersome to use with boilerplate type casting for each use that can be simplified within the library by doing type conversion before returning the value to an end user. Modifications: Many APIs that return a `RESPValue` now have overloads to provide a `RESPValueConvertible` type that the value will be turned into before being returned. For a few APIs that returned `RESPValue`, they did so as an Optional. Those APIs have been changed to always provide a `RESPValue` and return `.null` in cases where `nil` was returned. In addition, the `@inlinable` attribute has been removed from any non-generic command API. Result: Developers should have less code boilerplate for turning values from `RESPValue` to their desired type with many commands.
-
- 19 Mar, 2020 1 commit
-
-
Nathan Harris authored
-
- 20 Feb, 2020 3 commits
-
-
Nathan Harris authored
-
Nathan Harris authored
-
Nathan Harris authored
-
- 31 Dec, 2019 1 commit
-
-
Nathan Harris authored
Motivation: The SortedSet and List range commands (LTRIM, LRANGE, ZRANGE, etc.) are stringly-based and not flexible with Swift syntax. Modifications: - Add overloads of LTRIM that support the gambit of Range Standard Library types - Rework LRANGE to mirror LTRIM method signatures - Rework ZScore Range based commands to be more type-safe with `RedisZScoreBound` enum - Rework ZLex Range based commands to be more type-safe with `RedisZLexBound` enum - Rework ZCOUNT, ZLEXCOUNT, ZRANGE, ZREVRANGE, ZREMRANGEBYLEX, ZREMRANGEBYRANK, ZREMRANGEBYSCORE methods to be more type-safe and support Swift Range syntax Result: Working with SortedSet ranges should be much more type safe, and expressive with Swift's Range syntax.
-
- 28 Dec, 2019 2 commits
-
-
Nathan Harris authored
Motivation: While reviewing the API, the current design does not read well, and still has room for misunderstanding the actual end result of a ZADD operation. Modifications: - Rename `RedisSortedSetAddOption` to `RedisZaddInsertBehavior` and update cases to match desired use site syntax. - Add `RedisZaddReturnBehavior` enum to define how `zadd` should calculate the return value. - Update `zadd` and its overloads to support the two new enums in the form of `zadd(_:to:inserting:returning:)` Result: The more "Swifty" API will make it much more clear to developers at the call site what the actual behavior of the ZADD command will be.
-
Nathan Harris authored
Motivation: The goal is to have a strong-typed API for type-safety in arbitrary values, such as trying to use Int to represent time - as '3' could mean any unit of time, leaving many places for errors and bugs. Modifications: Switch all current APIs that accept a `timeout` argument to use `NIO.TimeAmount` instead of a plain `Int`. Result: Developers will have an easier time reasoning about their own code as to what values might mean when working with timeouts in Redis APIs.
-