Skip to content

Commit ff49974

Browse files
committed
address the remaining review comments
1 parent 5a437e3 commit ff49974

1 file changed

Lines changed: 17 additions & 12 deletions

File tree

tests/common/cluster_setup.rs

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ const STARTUP_TIMEOUT: Duration = Duration::from_secs(30);
4141
const CHANNEL_TIMEOUT: Duration = Duration::from_secs(60);
4242
const QUERY_TIMEOUT: Duration = Duration::from_secs(120);
4343
const CONNECTION_CLEANUP_TIMEOUT: Duration = Duration::from_millis(100);
44+
const PROCESS_CLEANUP_WAIT: Duration = Duration::from_secs(1);
45+
const WORKER_STARTUP_WAIT: Duration = Duration::from_secs(5);
46+
const READY_POLL_INTERVAL: Duration = Duration::from_millis(500);
47+
const WORKER_DISCOVERY_WAIT: Duration = Duration::from_secs(3);
48+
const SHUTDOWN_WAIT: Duration = Duration::from_secs(2);
4449

4550
/// Supported table formats for the cluster
4651
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -382,7 +387,7 @@ impl TestCluster {
382387
}
383388

384389
// Give processes time to die
385-
thread::sleep(Duration::from_secs(1));
390+
thread::sleep(PROCESS_CLEANUP_WAIT);
386391
Ok(())
387392
}
388393

@@ -441,7 +446,7 @@ impl TestCluster {
441446

442447
// Give workers time to start and be ready for connections
443448
println!(" Waiting for workers to be ready...");
444-
thread::sleep(Duration::from_secs(5));
449+
thread::sleep(WORKER_STARTUP_WAIT);
445450

446451
// Start proxy
447452
let proxy_port = self.config.proxy_port();
@@ -473,7 +478,7 @@ impl TestCluster {
473478

474479
let proxy = cmd.spawn()?;
475480
self.proxy_process = Some(proxy);
476-
self.is_running.store(true, Ordering::Relaxed);
481+
self.is_running.store(true, Ordering::SeqCst);
477482

478483
Ok(())
479484
}
@@ -500,14 +505,14 @@ impl TestCluster {
500505
);
501506
}
502507

503-
thread::sleep(Duration::from_millis(500));
508+
thread::sleep(READY_POLL_INTERVAL);
504509
}
505510

506511
Err("Cluster failed to become ready within timeout".into())
507512
}
508513

509514
async fn is_cluster_ready(&self) -> bool {
510-
if !self.is_running.load(Ordering::Relaxed) {
515+
if !self.is_running.load(Ordering::SeqCst) {
511516
return false;
512517
}
513518

@@ -570,7 +575,7 @@ impl TestCluster {
570575

571576
// Give additional time for worker discovery to complete
572577
println!(" Allowing time for worker discovery...");
573-
thread::sleep(Duration::from_secs(3));
578+
thread::sleep(WORKER_DISCOVERY_WAIT);
574579

575580
Ok(())
576581
}
@@ -583,7 +588,7 @@ impl TestCluster {
583588
&self,
584589
sql: &str,
585590
) -> Result<Vec<RecordBatch>, Box<dyn std::error::Error>> {
586-
if !self.is_running.load(Ordering::Relaxed) {
591+
if !self.is_running.load(Ordering::SeqCst) {
587592
return Err("Cluster is not running".into());
588593
}
589594

@@ -600,7 +605,7 @@ impl TestCluster {
600605
&self,
601606
sql: &str,
602607
) -> Result<Vec<RecordBatch>, Box<dyn std::error::Error>> {
603-
if !self.is_running.load(Ordering::Relaxed) {
608+
if !self.is_running.load(Ordering::SeqCst) {
604609
return Err("Cluster is not running".into());
605610
}
606611

@@ -701,13 +706,13 @@ impl TestCluster {
701706
/// Check if the cluster is running
702707
#[allow(dead_code)]
703708
pub fn is_running(&self) -> bool {
704-
self.is_running.load(Ordering::Relaxed)
709+
self.is_running.load(Ordering::SeqCst)
705710
}
706711
}
707712

708713
impl Drop for TestCluster {
709714
fn drop(&mut self) {
710-
if self.is_running.load(Ordering::Relaxed) {
715+
if self.is_running.load(Ordering::SeqCst) {
711716
println!("🧹 Shutting down integration test cluster...");
712717

713718
// Kill proxy
@@ -722,7 +727,7 @@ impl Drop for TestCluster {
722727
let _ = worker.wait();
723728
}
724729

725-
self.is_running.store(false, Ordering::Relaxed);
730+
self.is_running.store(false, Ordering::SeqCst);
726731
println!("✅ Cluster shutdown complete");
727732
}
728733
}
@@ -792,7 +797,7 @@ mod tests {
792797
drop(cluster);
793798

794799
// Give a moment for processes to die
795-
tokio::time::sleep(std::time::Duration::from_secs(2)).await;
800+
tokio::time::sleep(SHUTDOWN_WAIT).await;
796801

797802
// Verify the port is now available (should fail to connect)
798803
use std::net::TcpStream;

0 commit comments

Comments
 (0)