Design documentation for adding a raw-FFI thread manager #31
Conversation
| RustCore --> Redis | ||
| ``` | ||
|
|
||
| ## Decision to use Raw-FFI calls directly to Rust-Core for Golang Wrapper |
There was a problem hiding this comment.
I'd rather hold off on splitting until we have an idea of where these documents will be stored.
|
I think you need to change base to an integration branch |
acarbonetto
force-pushed
the
design-async-client-raw-ffi
branch
from
16d4a69 to
ad150b7
Compare
Signed-off-by: acarbonetto <[email protected]>
Signed-off-by: acarbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
acarbonetto
force-pushed
the
design-async-client-raw-ffi
branch
from
ad150b7 to
799b248
Compare
There was a problem hiding this comment.
@jonathanl-bq we should also mention the routing API implemented in the protobuf object. ie sending to specific cluster nodes.
We should also describe what type of verification will be performed on the wrapper level. We should consider having a type list for each supported command, and return an error early is types are violated for any supposed command. The execute / executeRaw functions won't have any verification.
docs/design-api.md
Outdated
| - There is additional overhead from marshalling data to and from protobuf, which could impact performance significantly | ||
|
|
||
| #### C Data Types | ||
| The wrapper language will pass commands, arguments, and configuration as C data types. |
There was a problem hiding this comment.
Can you please take few examples of complex commands with many arguments, like sorted set or list for example https://lettuce.io/core/release/api/io/lettuce/core/api/sync/RedisSortedSetCommands.html.
Lets try to better undersrtand it
There was a problem hiding this comment.
@asafpamzn also in the UDS solution we convert all passed arguments into a string array before passing it to the core. so there's no issue when passing complex commands.
There was a problem hiding this comment.
We need to verify that all RESP3 value types can be returned using C data types
There was a problem hiding this comment.
I think this comment is addressed now, since we will starting with RESP2 for now.
docs/design-api.md
Outdated
| Two different methods of sending requests will be supported. | ||
|
|
||
| ### No Redis Type Validation | ||
| We will expose an `executeRaw` method that does no validation of the input types or command on the client side, leaving it up to Redis to reject the request should it be malformed. This gives the user the flexibility to send any type of request they want, including ones not officially supported yet. |
There was a problem hiding this comment.
Can you please add an example
There was a problem hiding this comment.
Can you please explain what was the decision for type validation? There are two options
There was a problem hiding this comment.
We decided that we want to determine how much extra overhead protobuf adds to the product. Then compare with a custom payload in C. We can compare and share the solution for all raw-ffi solutions.
Signed-off-by: Andrew Carbonetto <[email protected]>
| Transactions will be handled by adding a list of `Command`s to the protobuf request. The response will be a `redis::Value::Bulk`, which can be marshalled into a C array of values before being passed from Rust to the wrapper language. The wrapper language is responsible for converting the array of results to its own native collection type. | ||
|
|
||
| Pros: | ||
| - We get to reuse the protobuf definitions, meaning fewer files to update if we make changes to the protobuf definitions |
There was a problem hiding this comment.
All Redis commands can be presented as a simple string array, so passing protobuf messages from the wrapper to the core adds unnecessary complication when we're talking about a raw FFI solution
There was a problem hiding this comment.
e.g. we'll have a generic execute_command function in rust that excepts a string array and all FFI functions from the wrapper will call it
docs/design-api.md
Outdated
| - There is additional overhead from marshalling data to and from protobuf, which could impact performance significantly | ||
|
|
||
| #### C Data Types | ||
| The wrapper language will pass commands, arguments, and configuration as C data types. |
There was a problem hiding this comment.
@asafpamzn also in the UDS solution we convert all passed arguments into a string array before passing it to the core. so there's no issue when passing complex commands.
docs/design-api.md
Outdated
| - There is additional overhead from marshalling data to and from protobuf, which could impact performance significantly | ||
|
|
||
| #### C Data Types | ||
| The wrapper language will pass commands, arguments, and configuration as C data types. |
There was a problem hiding this comment.
We need to verify that all RESP3 value types can be returned using C data types
Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
docs/design-api.md
Outdated
|
|
||
| ## API requirements: | ||
| - The API will be thread-safe. | ||
| - The API will accept as inputs all of [RESP2 types](https://github.com/redis/redis-specifications/blob/master/protocol/RESP2.md). |
There was a problem hiding this comment.
Lets add a comment that we are planning on supporting RESP3 types when they are released.
docs/design-api.md
Outdated
| ``` | ||
|
|
||
| ### With Redis Type Validation | ||
| We will expose separate methods for each supported command, and will attempt to validate the inputs for each of these methods. There will be a separate version of each method for transactions, as well as another version for Cluster Mode clients. We may leverage the compiler for the wrapper language to validate the types of the command arguments for statically typed languages. Since wrappers should be as lightweight as possible, we most likely will not be performing any checks for proper typing for non-statically typed languages. |
There was a problem hiding this comment.
"and will attempt to validate the inputs for each of these methods." -> the validation is done in the core/server itself
Co-authored-by: Andrew Carbonetto <[email protected]>
docs/design-api.md
Outdated
| We will be supporting all Redis commands. Commands with higher usage will be prioritized, as determined by usage numbers from AWS ElastiCache usage logs. | ||
|
|
||
| ## Type Validation | ||
| Two different methods of sending requests will be supported. |
There was a problem hiding this comment.
| Two different methods of sending requests will be supported. | |
| Two different methods of sending requests will be considered and compared, and we will go with the more performant solution. |
docs/design-api.md
Outdated
| Two different methods of sending requests will be supported. | ||
|
|
||
| ### No Redis Type Validation | ||
| We will expose an `executeRaw` method that does no validation of the input types or command on the client side, leaving it up to Redis to reject the request should it be malformed. This gives the user the flexibility to send any type of request they want, including ones not officially supported yet. |
There was a problem hiding this comment.
We decided that we want to determine how much extra overhead protobuf adds to the product. Then compare with a custom payload in C. We can compare and share the solution for all raw-ffi solutions.
Co-authored-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
| # Client Wrapper API design doc | ||
|
|
||
| ## API requirements: | ||
| - The API will be thread-safe. |
There was a problem hiding this comment.
may be use present instead of future?
| ## Command Interface | ||
|
|
||
| ### Unix Domain Socket solution | ||
| For clients based on Unix Domain Sockets (UDS), we will use the existing Rust-core protobuf messages for creating a connection, sending requests, and receiving responses. Supported commands are enumerated in the [protobuf definition for requests](../babushka-core/src/protobuf/redis_request.proto) and we will support them as they are added in the future. Note: the `CustomCommand` request type is also adequate for all commands. As defined in the [protobuf definition for responses](../babushka-core/src/protobuf/response.proto), client wrappers will receive data as a pointer, which can be passed to Rust to marshal the data back into the wrapper language’s native data types. In the future, we plan on optimizing this approach such that small data is returned as an object and large data is returned as a pointer in order to reduce the overhead of FFI calls in the case of small data. |
There was a problem hiding this comment.
Reword the first sentense - UDS is a transport, not a base for a client
| ``` | ||
|
|
||
| ## Routing Options | ||
| We will be supporting routing Redis requests to all nodes, all primary nodes, or a random node. For more specific routing to a node, we will also allow sending a request to a primary or replica node with a specified hash slot or key. When the wrapper given a key route, the key is passed to the Rust core, which will find the corresponding hash slot for it. |
There was a problem hiding this comment.
We will
or
We should
or
The client should/will/etc
Also: future or present? (here and below)
| ### Explicitly Supported Command | ||
| We will expose separate methods for each supported command. There will be a separate version of each method for transactions, as well as another version for Cluster Mode clients. For statically typed languages, we will leverage the compiler of the wrapper language to validate the types of the command arguments as much as possible. Since wrappers should be as lightweight as possible, we will be performing very few to no checks for proper typing for non-statically typed languages. | ||
|
|
||
| ## Errors |
There was a problem hiding this comment.
Reference where these errors came from
| We will expose separate methods for each supported command. There will be a separate version of each method for transactions, as well as another version for Cluster Mode clients. For statically typed languages, we will leverage the compiler of the wrapper language to validate the types of the command arguments as much as possible. Since wrappers should be as lightweight as possible, we will be performing very few to no checks for proper typing for non-statically typed languages. | ||
|
|
||
| ## Errors | ||
| ClosingError: Errors that report that the client has closed and is no longer usable. |
There was a problem hiding this comment.
| ClosingError: Errors that report that the client has closed and is no longer usable. | |
| `ClosingError`: Errors that report that the client has closed and is no longer usable. |
And below
| // submit three commands in a single transaction to Redis | ||
| Command getApplesRequest = Command.builder() | ||
| .requestType(GETSTRING) | ||
| .arguments(new String[]{apples"}) |
There was a problem hiding this comment.
| .arguments(new String[]{apples"}) | |
| .arguments(new String[]{"apples"}) |
| @@ -0,0 +1,240 @@ | |||
| # Babushka Core Wrappers | |||
There was a problem hiding this comment.
Please rename all references
| # Babushka Core Wrappers | |
| # Glide Core Wrappers |
| activate Wrapper | ||
| activate Client | ||
| Wrapper -)+ ffi: connect_to_redis | ||
| ffi -)+ manager: start_socket_listener(init_callback) |
There was a problem hiding this comment.
| ffi -)+ manager: start_socket_listener(init_callback) | |
| ffi -)+ manager: start_socket_listener |
There was a problem hiding this comment.
Rename file - it shows both raw FFI and UDS approaches
| Client -->> SocketListener: ClientUsageResult | ||
| SocketListener ->> Socket: write_result | ||
| Socket -->> SocketListener: | ||
| Wrapper ->> Socket: netty.read (protobuf.response) |
There was a problem hiding this comment.
This line is not correct. May be we need to split diagram into 2 or 3 ones: java to UDS, UDS to rust and java-uds-rust in zoom out mode.
…um indicating the value type
No description provided.