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.

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

Go: Implement connection logic in lib.rs #119

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c0cb6fb
Add Rust connection logic and example
jonathanl-bq Feb 27, 2024
22bbf74
Add error handling logic for connections
jonathanl-bq Feb 27, 2024
79343ff
Add license headers
jonathanl-bq Feb 27, 2024
2a92415
Refactor cArgs
jonathanl-bq Feb 27, 2024
3278942
Update API for client creation
jonathanl-bq Feb 29, 2024
2b258e5
Remove non-connection logic
jonathanl-bq Mar 4, 2024
6ccc6c1
Update lib.h
jonathanl-bq Mar 4, 2024
bdf9c19
Add additional documentation to lib.rs functions
jonathanl-bq Mar 5, 2024
4bd25cc
Properly free connection response and error
jonathanl-bq Mar 6, 2024
dc4df76
Run go fmt
jonathanl-bq Mar 6, 2024
cd5c2da
Add error for when user closes client before creating it
jonathanl-bq Mar 6, 2024
055e74b
Add doc comments about avoiding memory leaks
jonathanl-bq Mar 6, 2024
f23b01a
Allow unused Client fields for now
jonathanl-bq Mar 6, 2024
aa1fb08
Add formatting directive to fmt.Errorf
jonathanl-bq Mar 6, 2024
2fe5bdd
Replace callback implementations with TODO implementations
jonathanl-bq Mar 6, 2024
6289bfb
Resolve merge conflicts
jonathanl-bq Mar 6, 2024
53c8d7d
Remove RedisErrorFFI struct and use glide-core RequetErrorType instead
jonathanl-bq Mar 7, 2024
926e050
Update go/glide/lib.h
jonathanl-bq Mar 7, 2024
b9e61c8
Update go/glide/lib.h
jonathanl-bq Mar 7, 2024
70fb74b
Update go/src/lib.rs
jonathanl-bq Mar 7, 2024
db8959b
Update go/src/lib.rs
jonathanl-bq Mar 7, 2024
b32408c
Update documentation for create_client
jonathanl-bq Mar 7, 2024
478acd6
Adjust documentation regarding memory leaks
jonathanl-bq Mar 7, 2024
0e52d91
Make free_connection_response also free its error
jonathanl-bq Mar 7, 2024
a02912f
Update doc comment for free_connection_response
jonathanl-bq Mar 8, 2024
7ea7bdd
Re-expose free_error for the error callback to use and update docs
jonathanl-bq Mar 8, 2024
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
3 changes: 3 additions & 0 deletions go/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# Go compilation files
*.pb.go

# Example binary
examples/examples
jonathanl-bq marked this conversation as resolved.
Show resolved Hide resolved

reports
2 changes: 2 additions & 0 deletions go/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ redis = { path = "../submodules/redis-rs/redis", features = ["aio", "tokio-comp"
glide-core = { path = "../glide-core" }
tokio = { version = "^1", features = ["rt", "macros", "rt-multi-thread", "time"] }
protobuf = { version = "3.3.0", features = [] }
logger_core = {path = "../logger_core"}
tracing-subscriber = "0.3.16"

[profile.release]
lto = true
Expand Down
38 changes: 38 additions & 0 deletions go/examples/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/
package main

import (
"fmt"
"github.com/aws/glide-for-redis/go/glide/glide"
"github.com/aws/glide-for-redis/go/glide/protobuf"
"log"
)

func main() {
fmt.Println("Starting go-glide client...")
client := glide.GlideRedisClient{}
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved

// TODO: Update when configuration is implemented
request := &protobuf.ConnectionRequest{
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
TlsMode: protobuf.TlsMode_NoTls,
ClusterModeEnabled: false,
ReadFrom: protobuf.ReadFrom_Primary,
}
request.Addresses = append(
request.Addresses,
&protobuf.NodeAddress{Host: "localhost", Port: uint32(6379)},
)
connectionErr := client.ConnectToRedis(request)
if connectionErr != nil {
log.Fatal(connectionErr)
}

closeClientErr := client.CloseClient()
if closeClientErr != nil {
log.Fatal(closeClientErr)
}

fmt.Println("Disconnected from Redis")
}
75 changes: 75 additions & 0 deletions go/glide/glide.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/
package glide

/*
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
#cgo LDFLAGS: -L../target/release -lglide_rs
#include "lib.h"

void successCallback(uintptr_t channelPtr, char *message);
void failureCallback(uintptr_t channelPtr, char *errMessage, RequestErrorType errType);
*/
import "C"

import (
"fmt"
"github.com/aws/glide-for-redis/go/glide/protobuf"
"github.com/golang/protobuf/proto"
"unsafe"
)

type GlideRedisClient struct {
coreClient unsafe.Pointer
}

type payload struct {
value string
errMessage error
}

type RequestType uint32

type ErrorType uint32

const (
ClosingError = iota
RequestError
TimeoutError
ExecAbortError
ConnectionError
)

//export successCallback
func successCallback(channelPtr C.uintptr_t, message *C.char) {
// TODO: Implement when we implement the command logic
}

//export failureCallback
func failureCallback(channelPtr C.uintptr_t, errMessage *C.char, errType C.RequestErrorType) {
// TODO: Implement when we implement the command logic
}

func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.ConnectionRequest) error {
marshalledRequest, err := proto.Marshal(request)
if err != nil {
return fmt.Errorf("Failed to encode connection request: %v", err)
}
byteCount := len(marshalledRequest)
requestBytes := C.CBytes(marshalledRequest)
response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback))))
defer C.free_connection_response(response)
if response.error_message != nil {
return fmt.Errorf(C.GoString(response.error_message))
}
glideRedisClient.coreClient = response.conn_ptr
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

func (glideRedisClient *GlideRedisClient) CloseClient() error {
if glideRedisClient.coreClient == nil {
return fmt.Errorf("Cannot close glide client before it has been created.")

Choose a reason for hiding this comment

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

... or already closed

}
C.close_client(glideRedisClient.coreClient)

Choose a reason for hiding this comment

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

Suggested change
C.close_client(glideRedisClient.coreClient)
C.close_client(glideRedisClient.coreClient)
glideRedisClient.coreClient = nil

return nil
}
90 changes: 90 additions & 0 deletions go/glide/lib.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/

#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

typedef enum RequestErrorType {
Unspecified = 0,
ExecAbort = 1,
Timeout = 2,
Disconnect = 3,
} RequestErrorType;

/**
* The connection response.
*
* It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language.
jonathanl-bq marked this conversation as resolved.
Show resolved Hide resolved
*
* This struct should be freed using `free_connection_response` to avoid memory leaks.
*/
typedef struct ConnectionResponse {
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
const void *conn_ptr;
const char *error_message;
RequestErrorType error_type;
} ConnectionResponse;

/**
* Success callback that is called when a Redis command succeeds.
*/
typedef void (*SuccessCallback)(uintptr_t channel_address, const char *message);
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved

/**
* Failure callback that is called when a Redis command fails.
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
*
* `error` should be manually freed by calling `free_error` after this callback is invoked, otherwise a memory leak will occur.
*/
typedef void (*FailureCallback)(uintptr_t channel_address,
const char *error_message,
RequestErrorType error_type);

/**
* Creates a new client with the given configuration. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool.
*
* The returned `ConnectionResponse` should be manually freed by calling `free_connection_response`, otherwise a memory leak will occur. It should be freed whether or not an error occurs.
*
* # Safety
*
* * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes.
* * `connection_request_len` must not be greater than `isize::MAX`. See the safety documentation of [`std::slice::from_raw_parts`](https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html).
*/
const struct ConnectionResponse *create_client(const uint8_t *connection_request_bytes,
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
uintptr_t connection_request_len,
SuccessCallback success_callback,
FailureCallback failure_callback);

Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
/**
* Closes the given client, deallocating it from the heap.
*
* # Safety
*
* * `client_ptr` must be able to be safely casted to a valid `Box<Client>` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw).
* * `client_ptr` must not be null.
*/
void close_client(const void *client_ptr);

/**
* Deallocates a `ConnectionResponse`.
*
* This function also frees the contained error using `free_error`.
*
* # Safety
*
* * `connection_response_ptr` must be able to be safely casted to a valid `Box<ConnectionResponse>` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw).
* * `connection_response_ptr` must not be null.
* * The contained `error_message` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw).
*/
void free_connection_response(const struct ConnectionResponse *connection_response_ptr);

/**
* Deallocates an error message `CString`.
*
* # Safety
*
* * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw).
* * `error_msg_ptr` must not be null.
*/
void free_error(const char *error_msg_ptr);

Choose a reason for hiding this comment

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

Why re-expose?

Copy link
Author

Choose a reason for hiding this comment

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

I assumed that the failure callback would use it for the error it gets for commands. Not relevant for connection logic though, so I guess it can be reverted. Up to you, since you have all the changes in your branch now.

10 changes: 10 additions & 0 deletions go/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ require (
)

require (
github.com/BurntSushi/toml v1.2.1 // indirect
github.com/golang/protobuf v1.5.3 // indirect

Choose a reason for hiding this comment

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

Do you know why this is added? On my connect branch it is not needed since protobuf is already listed on line 7. If you do go mod tidy does this get removed?

github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/spf13/cobra v1.8.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/sync v0.6.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/tools v0.12.1-0.20230825192346-2191a27a6dc5 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/kr/pretty v0.3.1 // indirect
Expand Down
Loading
Loading