Skip to content

Commit

Permalink
Separate connection info from protobuf. (valkey-io#1088)
Browse files Browse the repository at this point in the history
* Separate connection info from protobuf.

This will allow the FFI clients to remove their dependency on protobuf.
Pros:
* faster compile times
* no need for protobuf dependency in wrappers.

* Update glide-core/src/client/types.rs

Co-authored-by: Aaron <[email protected]>

* round

---------

Co-authored-by: Shachar Langbeheim <[email protected]>
Co-authored-by: Aaron <[email protected]>
  • Loading branch information
3 people authored and cyip10 committed Jun 24, 2024
1 parent a90d785 commit 5bf1436
Show file tree
Hide file tree
Showing 22 changed files with 309 additions and 147 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/lint-rust/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ runs:
working-directory: ${{ inputs.cargo-toml-folder }}
shell: bash

# We run clippy without features
- run: cargo clippy --all-targets -- -D warnings
working-directory: ${{ inputs.cargo-toml-folder }}
shell: bash

- run: |
cargo update
cargo install cargo-deny
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ jobs:

- name: Run tests
working-directory: ./glide-core
run: cargo test -- --nocapture --test-threads=1 # TODO remove the concurrency limit after we fix test flakyness.
run: cargo test --all-features -- --nocapture --test-threads=1 # TODO remove the concurrency limit after we fix test flakyness.

- name: Run logger tests
working-directory: ./logger_core
run: cargo test -- --nocapture --test-threads=1
run: cargo test --all-features -- --nocapture --test-threads=1

- name: Check features
working-directory: ./glide-core
Expand Down
33 changes: 16 additions & 17 deletions benchmarks/rust/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ static GLOBAL: Jemalloc = Jemalloc;

use clap::Parser;
use futures::{self, future::join_all, stream, StreamExt};
use glide_core::{
client::Client,
connection_request::{ConnectionRequest, NodeAddress, TlsMode},
};
use glide_core::client::{Client, ConnectionRequest, NodeAddress, TlsMode};
use rand::{thread_rng, Rng};
use serde_json::Value;
use std::{
Expand Down Expand Up @@ -223,19 +220,21 @@ fn generate_random_string(length: usize) -> String {
}

async fn get_connection(args: &Args) -> Client {
let mut connection_request = ConnectionRequest::new();
connection_request.tls_mode = if args.tls {
TlsMode::SecureTls
} else {
TlsMode::NoTls
}
.into();
let mut address_info: NodeAddress = NodeAddress::new();
address_info.host = args.host.clone().into();
address_info.port = args.port;
connection_request.addresses.push(address_info);
connection_request.request_timeout = 2000;
connection_request.cluster_mode_enabled = args.cluster_mode_enabled;
let address_info: NodeAddress = NodeAddress {
host: args.host.clone(),
port: args.port as u16,
};
let connection_request = ConnectionRequest {
addresses: vec![address_info],
cluster_mode_enabled: args.cluster_mode_enabled,
request_timeout: Some(2000),
tls_mode: if args.tls {
Some(TlsMode::SecureTls)
} else {
Some(TlsMode::NoTls)
},
..Default::default()
};

glide_core::client::Client::new(connection_request)
.await
Expand Down
36 changes: 16 additions & 20 deletions csharp/lib/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/**
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/
use glide_core::connection_request;
use glide_core::{client::Client as GlideClient, connection_request::NodeAddress};
use glide_core::client;
use glide_core::client::Client as GlideClient;
use redis::{Cmd, FromRedisValue, RedisResult};
use std::{
ffi::{c_void, CStr, CString},
Expand All @@ -26,25 +26,21 @@ pub struct Client {
runtime: Runtime,
}

fn create_connection_request(
host: String,
port: u32,
use_tls: bool,
) -> connection_request::ConnectionRequest {
let mut address_info = NodeAddress::new();
address_info.host = host.to_string().into();
address_info.port = port;
let addresses_info = vec![address_info];
let mut connection_request = connection_request::ConnectionRequest::new();
connection_request.addresses = addresses_info;
connection_request.tls_mode = if use_tls {
connection_request::TlsMode::SecureTls
} else {
connection_request::TlsMode::NoTls
fn create_connection_request(host: String, port: u32, use_tls: bool) -> client::ConnectionRequest {
let address_info = client::NodeAddress {
host,
port: port as u16,
};
let addresses = vec![address_info];
client::ConnectionRequest {
addresses,
tls_mode: if use_tls {
Some(client::TlsMode::SecureTls)
} else {
Some(client::TlsMode::NoTls)
},
..Default::default()
}
.into();

connection_request
}

fn create_client_internal(
Expand Down
23 changes: 14 additions & 9 deletions glide-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,30 @@ authors = ["Amazon Web Services"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
bytes = "^1.3"
bytes = { version = "^1.3", optional = true }
futures = "^0.3"
redis = { path = "../submodules/redis-rs/redis", features = ["aio", "tokio-comp", "tokio-rustls-comp", "connection-manager","cluster", "cluster-async"] }
signal-hook = "^0.3"
signal-hook-tokio = {version = "^0.3", features = ["futures-v0_3"] }
signal-hook = { version = "^0.3", optional = true }
signal-hook-tokio = {version = "^0.3", features = ["futures-v0_3"], optional = true }
tokio = { version = "1", features = ["macros", "time"] }
logger_core = {path = "../logger_core"}
dispose = "0.5.0"
tokio-util = {version = "^0.7", features = ["rt"]}
num_cpus = "^1.15"
tokio-util = {version = "^0.7", features = ["rt"], optional = true}
num_cpus = { version = "^1.15", optional = true }
tokio-retry = "0.3.0"
protobuf = {version= "3", features = ["bytes", "with-bytes"]}
integer-encoding = "4.0.0"
protobuf = { version= "3", features = ["bytes", "with-bytes"], optional = true }
integer-encoding = { version = "4.0.0", optional = true }
thiserror = "1"
rand = "0.8.5"
rand = { version = "0.8.5", optional = true }
futures-intrusive = "0.5.0"
directories = "4.0"
directories = { version = "4.0", optional = true }
once_cell = "1.18.0"
arcstr = "1.1.5"
sha1_smol = "1.0.0"

[features]
socket-layer = ["directories", "integer-encoding", "num_cpus", "signal-hook", "signal-hook-tokio", "protobuf", "tokio-util", "bytes", "rand"]

[dev-dependencies]
rsevents = "0.3.1"
socket2 = "^0.5"
Expand All @@ -40,6 +43,8 @@ ctor = "0.2.2"
redis = { path = "../submodules/redis-rs/redis", features = ["tls-rustls-insecure"] }
iai-callgrind = "0.9"
tokio = { version = "1", features = ["rt-multi-thread"] }
glide-core = { path = ".", features = ["socket-layer"] } # always enable this feature in tests.


[build-dependencies]
protobuf-codegen = "3"
Expand Down
4 changes: 3 additions & 1 deletion glide-core/benches/memory_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ where
{
let runtime = Builder::new_current_thread().enable_all().build().unwrap();
runtime.block_on(async {
let client = Client::new(create_connection_request()).await.unwrap();
let client = Client::new(create_connection_request().into())
.await
.unwrap();
f(client).await;
});
}
Expand Down
8 changes: 7 additions & 1 deletion glide-core/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/

fn main() {
#[cfg(feature = "socket-layer")]
fn build_protobuf() {
let customization_options = protobuf_codegen::Customize::default()
.lite_runtime(false)
.tokio_bytes(true)
Expand All @@ -16,3 +17,8 @@ fn main() {
.customize(customization_options)
.run_from_script();
}

fn main() {
#[cfg(feature = "socket-layer")]
build_protobuf();
}
Loading

0 comments on commit 5bf1436

Please sign in to comment.