From 5cf5edd5feb0f8c490a3569809931fcc5fa82ba7 Mon Sep 17 00:00:00 2001 From: Jeff Biseda Date: Mon, 26 Jun 2023 10:31:55 -0700 Subject: [PATCH] unnecessary error conversions in core (#32257) --- core/src/ancestor_hashes_service.rs | 12 ++++--- core/src/repair_service.rs | 53 +++++++++-------------------- core/src/serve_repair.rs | 12 ++++--- core/src/tower1_7_14.rs | 4 +-- 4 files changed, 33 insertions(+), 48 deletions(-) diff --git a/core/src/ancestor_hashes_service.rs b/core/src/ancestor_hashes_service.rs index 33c41bd50a..b66d26f61b 100644 --- a/core/src/ancestor_hashes_service.rs +++ b/core/src/ancestor_hashes_service.rs @@ -8,13 +8,12 @@ use { packet_threshold::DynamicPacketToProcessThreshold, repair_service::{AncestorDuplicateSlotsSender, RepairInfo, RepairStatsGroup}, replay_stage::DUPLICATE_THRESHOLD, - result::{Error, Result}, serve_repair::{ AncestorHashesRepairType, AncestorHashesResponse, RepairProtocol, ServeRepair, }, }, bincode::serialize, - crossbeam_channel::{unbounded, Receiver, Sender}, + crossbeam_channel::{unbounded, Receiver, RecvTimeoutError, Sender}, dashmap::{mapref::entry::Entry::Occupied, DashMap}, solana_gossip::{cluster_info::ClusterInfo, ping_pong::Pong}, solana_ledger::blockstore::Blockstore, @@ -246,8 +245,11 @@ impl AncestorHashesService { &ancestor_socket, ); match result { - Err(Error::RecvTimeout(_)) | Ok(_) => {} - Err(err) => info!("ancestors hashes responses listener error: {:?}", err), + Ok(_) | Err(RecvTimeoutError::Timeout) => (), + Err(RecvTimeoutError::Disconnected) => { + info!("ancestors hashes responses listener disconnected"); + return; + } }; if exit.load(Ordering::Relaxed) { return; @@ -274,7 +276,7 @@ impl AncestorHashesService { retryable_slots_sender: &RetryableSlotsSender, keypair: &Keypair, ancestor_socket: &UdpSocket, - ) -> Result<()> { + ) -> Result<(), RecvTimeoutError> { let timeout = Duration::new(1, 0); let mut packet_batches = vec![response_receiver.recv_timeout(timeout)?]; let mut total_packets = packet_batches[0].len(); diff --git a/core/src/repair_service.rs b/core/src/repair_service.rs index bf4f05096f..59703f3866 100644 --- a/core/src/repair_service.rs +++ b/core/src/repair_service.rs @@ -2,7 +2,7 @@ //! regularly finds missing shreds in the ledger and sends repair requests for those shreds #[cfg(test)] use { - crate::duplicate_repair_status::DuplicateSlotRepairStatus, solana_ledger::shred::Nonce, + crate::duplicate_repair_status::DuplicateSlotRepairStatus, solana_sdk::clock::DEFAULT_MS_PER_SLOT, }; use { @@ -670,7 +670,7 @@ impl RepairService { blockstore: &Blockstore, max_repairs: usize, repair_range: &RepairSlotRange, - ) -> crate::result::Result> { + ) -> Vec { // Slot height and shred indexes for shreds we want to repair let mut repairs: Vec = vec![]; for slot in repair_range.start..=repair_range.end { @@ -695,7 +695,7 @@ impl RepairService { repairs.extend(new_repairs); } - Ok(repairs) + repairs } #[cfg(test)] @@ -750,20 +750,23 @@ impl RepairService { let mut outstanding_requests = outstanding_requests.write().unwrap(); for repair_type in repairs { let nonce = outstanding_requests.add_request(repair_type, timestamp()); - if let Err(e) = Self::serialize_and_send_request( + + match serve_repair.map_repair_request( &repair_type, - repair_socket, &repair_pubkey, - &repair_addr, - serve_repair, repair_stats, nonce, identity_keypair, ) { - info!( - "repair req send_to {} ({}) error {:?}", - repair_pubkey, repair_addr, e - ); + Ok(req) => { + if let Err(e) = repair_socket.send_to(&req, repair_addr) { + info!( + "repair req send_to {} ({}) error {:?}", + repair_pubkey, repair_addr, e + ); + } + } + Err(e) => info!("map_repair_request err={e}"), } } true @@ -776,28 +779,6 @@ impl RepairService { }) } - #[cfg(test)] - fn serialize_and_send_request( - repair_type: &ShredRepairType, - repair_socket: &UdpSocket, - repair_pubkey: &Pubkey, - to: &SocketAddr, - serve_repair: &ServeRepair, - repair_stats: &mut RepairStats, - nonce: Nonce, - identity_keypair: &Keypair, - ) -> crate::result::Result<()> { - let req = serve_repair.map_repair_request( - repair_type, - repair_pubkey, - repair_stats, - nonce, - identity_keypair, - )?; - repair_socket.send_to(&req, to)?; - Ok(()) - } - #[cfg(test)] fn update_duplicate_slot_repair_addr( slot: Slot, @@ -1115,8 +1096,7 @@ mod test { &blockstore, std::usize::MAX, &repair_slot_range, - ) - .unwrap(), + ), expected ); } @@ -1163,8 +1143,7 @@ mod test { &blockstore, std::usize::MAX, &repair_slot_range, - ) - .unwrap(), + ), expected ); } diff --git a/core/src/serve_repair.rs b/core/src/serve_repair.rs index 6cc133217f..d2eea944db 100644 --- a/core/src/serve_repair.rs +++ b/core/src/serve_repair.rs @@ -8,6 +8,7 @@ use { result::{Error, Result}, }, bincode::serialize, + crossbeam_channel::RecvTimeoutError, lru::LruCache, rand::{ distributions::{Distribution, WeightedError, WeightedIndex}, @@ -628,7 +629,7 @@ impl ServeRepair { response_sender: &PacketBatchSender, stats: &mut ServeRepairStats, data_budget: &DataBudget, - ) -> Result<()> { + ) -> std::result::Result<(), RecvTimeoutError> { //TODO cache connections let timeout = Duration::new(1, 0); let mut reqs_v = vec![requests_receiver.recv_timeout(timeout)?]; @@ -831,8 +832,11 @@ impl ServeRepair { &data_budget, ); match result { - Err(Error::RecvTimeout(_)) | Ok(_) => {} - Err(err) => info!("repair listener error: {:?}", err), + Ok(_) | Err(RecvTimeoutError::Timeout) => {} + Err(RecvTimeoutError::Disconnected) => { + info!("repair listener disconnected"); + return; + } }; if exit.load(Ordering::Relaxed) { return; @@ -1367,7 +1371,7 @@ impl ServeRepair { mod tests { use { super::*, - crate::{repair_response, result::Error}, + crate::repair_response, solana_gossip::{contact_info::ContactInfo, socketaddr, socketaddr_any}, solana_ledger::{ blockstore::make_many_slot_entries, diff --git a/core/src/tower1_7_14.rs b/core/src/tower1_7_14.rs index 823b678f24..982ffffb60 100644 --- a/core/src/tower1_7_14.rs +++ b/core/src/tower1_7_14.rs @@ -1,5 +1,5 @@ use { - crate::consensus::{SwitchForkDecision, TowerError}, + crate::consensus::{Result, SwitchForkDecision, TowerError}, solana_sdk::{ clock::Slot, hash::Hash, @@ -46,7 +46,7 @@ pub struct SavedTower1_7_14 { } impl SavedTower1_7_14 { - pub fn new(tower: &Tower1_7_14, keypair: &T) -> Result { + pub fn new(tower: &Tower1_7_14, keypair: &T) -> Result { let node_pubkey = keypair.pubkey(); if tower.node_pubkey != node_pubkey { return Err(TowerError::WrongTower(format!(