Skip to content

Commit 3451846

Browse files
author
avifenesh
committed
Update dependencies and refactor random string generation
Signed-off-by: avifenesh <[email protected]>
1 parent 4de9ae4 commit 3451846

File tree

7 files changed

+65
-74
lines changed

7 files changed

+65
-74
lines changed

glide-core/Cargo.toml

+18-18
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ authors = ["Valkey GLIDE Maintainers"]
99

1010
[dependencies]
1111
bytes = "1"
12-
futures = "^0.3"
12+
futures = "0.3"
1313
redis = { path = "./redis-rs/redis", features = [
1414
"aio",
1515
"tokio-comp",
@@ -21,27 +21,27 @@ redis = { path = "./redis-rs/redis", features = [
2121
telemetrylib = { path = "./telemetry" }
2222
tokio = { version = "1", features = ["macros", "time"] }
2323
logger_core = { path = "../logger_core" }
24-
dispose = "0.5.0"
25-
tokio-util = { version = "^0.7", features = ["rt"], optional = true }
26-
num_cpus = { version = "^1.15", optional = true }
27-
tokio-retry2 = {version = "0.5", features = ["jitter"]}
24+
dispose = "0.5"
25+
tokio-util = { version = "0.7", features = ["rt"], optional = true }
26+
num_cpus = { version = "1", optional = true }
27+
tokio-retry2 = { version = "0.5", features = ["jitter"] }
2828

2929
protobuf = { version = "3", features = [
3030
"bytes",
3131
"with-bytes",
3232
], optional = true }
33-
integer-encoding = { version = "4.0.0", optional = true }
34-
thiserror = "1"
35-
rand = { version = "0.8.5" }
36-
futures-intrusive = "0.5.0"
37-
directories = { version = "5.0", optional = true }
38-
once_cell = "1.18.0"
39-
sha1_smol = "1.0.0"
40-
nanoid = "0.4.0"
41-
async-trait = { version = "0.1.24" }
33+
integer-encoding = { version = "4", optional = true }
34+
thiserror = "2"
35+
rand = "0.9"
36+
futures-intrusive = "0.5"
37+
directories = { version = "6", optional = true }
38+
once_cell = "1"
39+
sha1_smol = "1"
40+
nanoid = "0.4"
41+
async-trait = { version = "0.1" }
4242
serde_json = "1"
4343
serde = { version = "1", features = ["derive"] }
44-
versions = "6.3"
44+
versions = "6"
4545

4646
[features]
4747
socket-layer = [
@@ -57,11 +57,11 @@ standalone_heartbeat = []
5757
rsevents = "0.3.1"
5858
socket2 = "^0.5"
5959
tempfile = "3.3.0"
60-
rstest = "^0.23"
60+
rstest = "^0.24"
6161
serial_test = "3"
6262
criterion = { version = "^0.5", features = ["html_reports", "async_tokio"] }
63-
which = "6"
64-
ctor = "0.2.2"
63+
which = "7"
64+
ctor = "0.3"
6565
redis = { path = "./redis-rs/redis", features = ["tls-rustls-insecure"] }
6666
iai-callgrind = "0.14"
6767
tokio = { version = "1", features = ["rt-multi-thread"] }

glide-core/benches/rotating_buffer_benchmark.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use glide_core::{
1111
};
1212
use integer_encoding::VarInt;
1313
use protobuf::Message;
14-
use rand::{distributions::Alphanumeric, Rng};
14+
use rand::{distr::Alphanumeric, Rng};
1515

1616
fn benchmark(
1717
c: &mut Criterion,
@@ -102,7 +102,7 @@ fn benchmark_split_data(
102102
}
103103

104104
fn generate_random_string(length: usize) -> bytes::Bytes {
105-
let s: String = rand::thread_rng()
105+
let s: String = rand::rng()
106106
.sample_iter(&Alphanumeric)
107107
.take(length)
108108
.map(char::from)

glide-core/src/client/standalone_client.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use crate::retry_strategies::RetryStrategy;
99
use futures::{future, stream, StreamExt};
1010
use logger_core::log_debug;
1111
use logger_core::log_warn;
12-
use rand::Rng;
1312
use redis::aio::ConnectionLike;
1413
use redis::cluster_routing::{self, is_readonly_cmd, ResponsePolicy, Routable, RoutingInfo};
1514
use redis::{PushInfo, RedisError, RedisResult, Value};
@@ -125,7 +124,7 @@ impl StandaloneClient {
125124
let tls_mode = connection_request.tls_mode;
126125
let node_count = connection_request.addresses.len();
127126
// randomize pubsub nodes, maybe a batter option is to always use the primary
128-
let pubsub_node_index = rand::thread_rng().gen_range(0..node_count);
127+
let pubsub_node_index = rand::random_range(0..node_count);
129128
let pubsub_addr = &connection_request.addresses[pubsub_node_index];
130129
let discover_az = matches!(
131130
connection_request.read_from,

glide-core/src/rotating_buffer.rs

+13-14
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,17 @@ impl RotatingBuffer {
2929
let start_pos = prev_position + bytes_read;
3030
if (start_pos + request_len as usize) > buffer_len {
3131
break;
32-
} else {
33-
match T::parse_from_tokio_bytes(
34-
&buffer.slice(start_pos..start_pos + request_len as usize),
35-
) {
36-
Ok(request) => {
37-
prev_position += request_len as usize + bytes_read;
38-
results.push(request);
39-
}
40-
Err(err) => {
41-
log_error("parse input", format!("Failed to parse request: {err}"));
42-
return Err(err.into());
43-
}
32+
}
33+
match T::parse_from_tokio_bytes(
34+
&buffer.slice(start_pos..start_pos + request_len as usize),
35+
) {
36+
Ok(request) => {
37+
prev_position += request_len as usize + bytes_read;
38+
results.push(request);
39+
}
40+
Err(err) => {
41+
log_error("parse input", format!("Failed to parse request: {err}"));
42+
return Err(err.into());
4443
}
4544
}
4645
} else {
@@ -68,7 +67,7 @@ mod tests {
6867
use crate::command_request::{command, command_request};
6968
use crate::command_request::{Command, CommandRequest, RequestType};
7069
use bytes::BufMut;
71-
use rand::{distributions::Alphanumeric, Rng};
70+
use rand::{distr::Alphanumeric, Rng};
7271
use rstest::rstest;
7372

7473
fn write_length(buffer: &mut BytesMut, length: u32) {
@@ -161,7 +160,7 @@ mod tests {
161160
}
162161

163162
fn generate_random_string(length: usize) -> String {
164-
rand::thread_rng()
163+
rand::rng()
165164
.sample_iter(&Alphanumeric)
166165
.take(length)
167166
.map(char::from)

glide-core/src/socket_listener.rs

+26-28
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use crate::errors::{error_message, error_type, RequestErrorType};
1111
use crate::response;
1212
use crate::response::Response;
1313
use bytes::Bytes;
14-
use directories::BaseDirs;
1514
use logger_core::{log_debug, log_error, log_info, log_trace, log_warn};
1615
use once_cell::sync::Lazy;
1716
use protobuf::{Chars, Message};
@@ -24,8 +23,8 @@ use std::cell::Cell;
2423
use std::collections::HashSet;
2524
use std::ptr::from_mut;
2625
use std::rc::Rc;
26+
use std::str;
2727
use std::sync::RwLock;
28-
use std::{env, str};
2928
use std::{io, thread};
3029
use thiserror::Error;
3130
use tokio::net::{UnixListener, UnixStream};
@@ -39,7 +38,8 @@ use ClosingReason::*;
3938
use PipeListeningResult::*;
4039

4140
/// The socket file name
42-
const SOCKET_FILE_NAME: &str = "glide-socket";
41+
const SOCKET_FILE_NAME: &str = "glide-socket.sock";
42+
const SOCKET_FOLDER: &str = "valkey-glide";
4343

4444
/// The maximum length of a request's arguments to be passed as a vector of
4545
/// strings instead of a pointer
@@ -569,7 +569,13 @@ async fn handle_requests(
569569

570570
pub fn close_socket(socket_path: &String) {
571571
log_info("close_socket", format!("closing socket at {socket_path}"));
572-
let _ = std::fs::remove_file(socket_path);
572+
// On Unix systems, the files are "unlinked" but not deleted until all references to the file are closed.
573+
// Based on the officials docs of `rust std::fs::remove_file: https://doc.rust-lang.org/std/fs/fn.remove_file.html`,
574+
// "Note that there is no guarantee that the file is immediately deleted (e.g., depending on platform, other open file descriptors may prevent immediate removal)."
575+
// So, we rename the file to a new name and then remove it, to avoid any issues with the file being unlinked but not deleted, and the system is closed before the file is deleted.
576+
// As we saw with dockers being closed on immediate shutdown of the program.
577+
let _ = std::fs::rename(socket_path, format!("{socket_path}.closed"));
578+
let _ = std::fs::remove_file(format!("{socket_path}.closed"));
573579
}
574580

575581
async fn create_client(
@@ -582,6 +588,8 @@ async fn create_client(
582588
Err(err) => return Err(ClientCreationError::ConnectionError(err)),
583589
};
584590
write_result(Ok(Value::Okay), 0, writer).await?;
591+
592+
log_info("create_client", "Client created successfully");
585593
Ok(client)
586594
}
587595

@@ -780,32 +788,22 @@ struct ClosingError {
780788
err_message: String,
781789
}
782790

783-
/// Get the socket full path.
784-
/// The socket file name will contain the process ID and will try to be saved into the user's runtime directory
785-
/// (e.g. /run/user/1000) in Unix systems. If the runtime dir isn't found, the socket file will be saved to the temp dir.
786-
/// For Windows, the socket file will be saved to %AppData%\Local.
787-
pub fn get_socket_path_from_name(socket_name: String) -> String {
788-
let base_dirs = BaseDirs::new().expect("Failed to create BaseDirs");
789-
let tmp_dir;
790-
let folder = if cfg!(windows) {
791-
base_dirs.data_local_dir()
792-
} else {
793-
base_dirs.runtime_dir().unwrap_or({
794-
tmp_dir = env::temp_dir();
795-
tmp_dir.as_path()
796-
})
797-
};
798-
folder
799-
.join(socket_name)
800-
.into_os_string()
801-
.into_string()
802-
.expect("Couldn't create socket path")
803-
}
804-
805791
/// Get the socket path as a string
806792
pub fn get_socket_path() -> String {
807-
let socket_name = format!("{}-{}", SOCKET_FILE_NAME, std::process::id());
808-
get_socket_path_from_name(socket_name)
793+
// Remove the socket folder if it exists, to ensure there are no conflicts with the socket file
794+
// that will be created.
795+
let tmp_dir = std::env::temp_dir();
796+
let socket_folder = tmp_dir
797+
.join(SOCKET_FOLDER)
798+
.join(std::process::id().to_string());
799+
800+
let _ = std::fs::remove_dir_all(&socket_folder);
801+
802+
let _ = std::fs::create_dir_all(&socket_folder);
803+
socket_folder
804+
.join(SOCKET_FILE_NAME)
805+
.to_string_lossy()
806+
.into_owned()
809807
}
810808

811809
/// This function is exposed only for the sake of testing with a nonstandard `socket_path`.

glide-core/tests/test_socket_listener.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -515,11 +515,9 @@ mod socket_listener {
515515

516516
#[rstest]
517517
#[timeout(SHORT_STANDALONE_TEST_TIMEOUT)]
518+
#[cfg(test)]
518519
fn test_working_after_socket_listener_was_dropped() {
519-
let socket_path = get_socket_path_from_name(format!(
520-
"{}_test_working_after_socket_listener_was_dropped",
521-
std::process::id()
522-
));
520+
let socket_path = get_socket_path();
523521
close_socket(&socket_path);
524522
// create a socket listener and drop it, to simulate a panic in a previous iteration.
525523
Builder::new_current_thread()
@@ -556,10 +554,7 @@ mod socket_listener {
556554
#[rstest]
557555
#[timeout(SHORT_STANDALONE_TEST_TIMEOUT)]
558556
fn test_multiple_listeners_competing_for_the_socket() {
559-
let socket_path = get_socket_path_from_name(format!(
560-
"{}_test_multiple_listeners_competing_for_the_socket",
561-
std::process::id()
562-
));
557+
let socket_path = get_socket_path();
563558
close_socket(&socket_path);
564559
let server = Arc::new(RedisServer::new(ServerType::Tcp { tls: false }));
565560

glide-core/tests/utilities/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use glide_core::{
77
connection_request::{self, AuthenticationInfo, NodeAddress, ProtocolVersion},
88
};
99
use once_cell::sync::Lazy;
10-
use rand::{distributions::Alphanumeric, Rng};
10+
use rand::{distr::Alphanumeric, Rng};
1111
use redis::{
1212
cluster_routing::{MultipleNodeRoutingInfo, RoutingInfo},
1313
ConnectionAddr, GlideConnectionOptions, PushInfo, RedisConnectionInfo, RedisResult, Value,
@@ -517,7 +517,7 @@ pub fn get_address_info(address: &ConnectionAddr) -> NodeAddress {
517517
}
518518

519519
pub fn generate_random_string(length: usize) -> String {
520-
rand::thread_rng()
520+
rand::rng()
521521
.sample_iter(&Alphanumeric)
522522
.take(length)
523523
.map(char::from)

0 commit comments

Comments
 (0)