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

opt: optimize cluster identification #3032

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod caches;
mod farms;

use crate::commands::cluster::controller::caches::maintain_caches;
use crate::commands::cluster::controller::farms::{maintain_farms, FarmIndex};
use crate::commands::cluster::controller::farms::maintain_farms;
use crate::commands::shared::derive_libp2p_keypair;
use crate::commands::shared::network::{configure_network, NetworkArgs};
use anyhow::anyhow;
Expand All @@ -20,6 +20,7 @@ use std::sync::Arc;
use std::time::Duration;
use subspace_core_primitives::crypto::kzg::{embedded_kzg_settings, Kzg};
use subspace_farmer::cluster::controller::controller_service;
use subspace_farmer::cluster::farmer::FarmIndex;
use subspace_farmer::cluster::nats_client::NatsClient;
use subspace_farmer::farm::plotted_pieces::PlottedPieces;
use subspace_farmer::farmer_cache::FarmerCache;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use std::sync::Arc;
use std::time::Instant;
use subspace_core_primitives::{Blake3Hash, SectorIndex};
use subspace_farmer::cluster::controller::ClusterControllerFarmerIdentifyBroadcast;
use subspace_farmer::cluster::farmer::{ClusterFarm, ClusterFarmerIdentifyFarmBroadcast};
use subspace_farmer::cluster::farmer::{
ClusterFarm, ClusterFarmerIdentifyFarmBroadcast, FarmIndex,
};
use subspace_farmer::cluster::nats_client::NatsClient;
use subspace_farmer::farm::plotted_pieces::PlottedPieces;
use subspace_farmer::farm::{Farm, FarmId, SectorPlottingDetails, SectorUpdate};
Expand All @@ -32,8 +34,6 @@ use tracing::{error, info, trace, warn};
type AddRemoveFuture<'a> =
Pin<Box<dyn Future<Output = Option<(FarmIndex, oneshot::Receiver<()>, ClusterFarm)>> + 'a>>;

pub(super) type FarmIndex = u16;
Copy link
Member

Choose a reason for hiding this comment

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

This was meant to be a generic, cluster doesn't need to know what the type is, we could have used u8, but decided to allow for more than 256 farms in cluster mode. Why was this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just move to subspace_farmer::cluster::farmer, because I want to use it in ClusterFarmerIdentifyBroadcast

Copy link
Member

Choose a reason for hiding this comment

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

There should be no need to send it in there, it seems to be an artifact of ID derivation mechanism


#[derive(Debug)]
struct KnownFarm {
farm_id: FarmId,
Expand Down
2 changes: 2 additions & 0 deletions crates/subspace-farmer/src/cluster/farmer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ use tracing::{debug, error, trace, warn};
const BROADCAST_NOTIFICATIONS_BUFFER: usize = 1000;
const MIN_FARMER_IDENTIFICATION_INTERVAL: Duration = Duration::from_secs(1);

/// Type alias for farm index used by cluster.
pub type FarmIndex = u16;
type Handler<A> = Bag<HandlerFn<A>, A>;

/// Broadcast with identification details by farmers
Expand Down