fix: Remove now-unused ServerId. Fixes #4451

clap_blocks/src/lib.rs

@@ -7,6 +7,5 @@ pub mod ingester;
 pub mod object_store;
 pub mod querier;
 pub mod run_config;
-pub mod server_id;
 pub mod socket_addr;
 pub mod write_buffer;

clap_blocks/src/run_config.rs

@@ -1,7 +1,7 @@
 use trace_exporters::TracingConfig;
 use trogging::cli::LoggingConfig;
 
-use crate::{object_store::ObjectStoreConfig, server_id::ServerIdConfig, socket_addr::SocketAddr};
+use crate::{object_store::ObjectStoreConfig, socket_addr::SocketAddr};
 
 /// The default bind address for the HTTP API.
 pub const DEFAULT_API_BIND_ADDR: &str = "127.0.0.1:8080";
@@ -20,10 +20,6 @@ pub struct RunConfig {
     #[clap(flatten)]
     pub(crate) tracing_config: TracingConfig,
 
-    /// object store config
-    #[clap(flatten)]
-    pub(crate) server_id_config: ServerIdConfig,
-
     /// The address on which IOx will serve HTTP API requests.
     #[clap(
     long = "--api-bind",
@@ -69,21 +65,11 @@ impl RunConfig {
         &mut self.tracing_config
     }
 
-    /// Get a reference to the run config's server id config.
-    pub fn server_id_config(&self) -> &ServerIdConfig {
-        &self.server_id_config
-    }
-
     /// Get a reference to the run config's logging config.
     pub fn logging_config(&self) -> &LoggingConfig {
         &self.logging_config
     }
 
-    /// Get a mutable reference to the run config's server id config.
-    pub fn server_id_config_mut(&mut self) -> &mut ServerIdConfig {
-        &mut self.server_id_config
-    }
-
     /// set the http bind address
     pub fn with_http_bind_address(mut self, http_bind_address: SocketAddr) -> Self {
         self.http_bind_address = http_bind_address;
@@ -108,9 +94,6 @@ impl RunConfig {
         Self {
             logging_config,
             tracing_config,
-            // TODO: server_id isn't used in NG; this field should be removed when OG is removed
-            // https://github.com/influxdata/influxdb_iox/issues/4451
-            server_id_config: ServerIdConfig { server_id: None },
             http_bind_address,
             grpc_bind_address,
             max_http_request_size,

clap_blocks/src/server_id.rs

@@ -1,14 +0,0 @@
-use data_types::server_id::ServerId;
-
-/// CLI config for server ID.
-#[derive(Debug, Clone, clap::Parser)]
-pub struct ServerIdConfig {
-    /// The identifier for the server.
-    ///
-    /// Used for writing to object storage and as an identifier that is added to
-    /// replicated writes, write buffer segments, and Chunks. Must be unique in
-    /// a group of connected or semi-connected IOx servers. Must be a nonzero
-    /// number that can be represented by a 32-bit unsigned integer.
-    #[clap(long = "--server-id", env = "INFLUXDB_IOX_ID")]
-    pub server_id: Option<ServerId>,
-}

data_types/src/lib.rs

@@ -15,7 +15,6 @@ pub mod consistent_hasher;
 pub mod error;
 pub mod job;
 pub mod partition_metadata;
-pub mod server_id;
 pub mod timestamp;
 pub mod write_buffer;
 pub mod write_summary;

data_types/src/server_id.rs

@@ -1,106 +0,0 @@
-use snafu::{OptionExt, ResultExt, Snafu};
-use std::{
-    convert::TryFrom,
-    fmt,
-    num::{NonZeroU32, ParseIntError},
-    str::FromStr,
-};
-
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
-pub struct ServerId(NonZeroU32);
-
-impl ServerId {
-    pub fn new(id: NonZeroU32) -> Self {
-        Self(id)
-    }
-
-    pub fn get(&self) -> NonZeroU32 {
-        self.0
-    }
-
-    pub fn get_u32(&self) -> u32 {
-        self.0.get()
-    }
-}
-
-impl FromStr for ServerId {
-    type Err = Error;
-
-    fn from_str(value: &str) -> Result<Self, Self::Err> {
-        let value: u32 = value.parse().context(UnableToParseSnafu { value })?;
-        Self::try_from(value)
-    }
-}
-
-impl TryFrom<u32> for ServerId {
-    type Error = Error;
-
-    fn try_from(value: u32) -> Result<Self, Self::Error> {
-        NonZeroU32::new(value)
-            .map(Self)
-            .context(ValueMayNotBeZeroSnafu)
-            .map_err(Into::into)
-    }
-}
-
-impl fmt::Display for ServerId {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.fmt(f)
-    }
-}
-
-#[derive(Debug, Snafu)]
-pub struct Error(InnerError);
-
-#[derive(Debug, Snafu)]
-enum InnerError {
-    #[snafu(display("The server ID may not be zero"))]
-    ValueMayNotBeZero,
-
-    #[snafu(display("Could not parse {} as a non-zero 32-bit unsigned number", value))]
-    UnableToParse {
-        source: ParseIntError,
-        value: String,
-    },
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn cannot_be_zero() {
-        assert!(matches!(
-            ServerId::try_from(0),
-            Err(Error(InnerError::ValueMayNotBeZero))
-        ));
-    }
-
-    #[test]
-    fn can_be_nonzero() {
-        let value = 2;
-        let server_id = ServerId::try_from(value).unwrap();
-        assert_eq!(server_id.get_u32(), value);
-    }
-
-    #[test]
-    fn can_be_parsed_from_a_string() {
-        assert!(matches!(
-            "0".parse::<ServerId>(),
-            Err(Error(InnerError::ValueMayNotBeZero)),
-        ));
-        assert!(matches!(
-            "moo".parse::<ServerId>(),
-            Err(Error(InnerError::UnableToParse { source: _, value })) if value == "moo",
-        ));
-
-        let server_id = "1337".parse::<ServerId>().unwrap();
-        assert_eq!(server_id.get_u32(), 1337);
-    }
-
-    #[test]
-    fn can_be_displayed() {
-        let server_id = ServerId::try_from(42).unwrap();
-        assert_eq!("42", format!("{}", server_id));
-    }
-}

generated_types/src/google.rs

@@ -233,7 +233,6 @@ pub enum ResourceType {
     DatabaseUuid,
     Job,
     Router,
-    ServerId,
     Unknown(String),
 }
 
@@ -247,7 +246,6 @@ impl ResourceType {
             Self::Chunk => "chunk",
             Self::Job => "job",
             Self::Router => "router",
-            Self::ServerId => "server_id",
             Self::Unknown(unknown) => unknown,
         }
     }
@@ -263,7 +261,6 @@ impl From<String> for ResourceType {
             "chunk" => Self::Chunk,
             "job" => Self::Job,
             "router" => Self::Router,
-            "server_id" => Self::ServerId,
             _ => Self::Unknown(s),
         }
     }
@@ -417,8 +414,6 @@ pub fn decode_not_found(status: &tonic::Status) -> impl Iterator<Item = NotFound
 /// prevents performing the requested operation
 #[derive(Debug, Clone, PartialEq)]
 pub enum PreconditionViolation {
-    /// Server ID not set
-    ServerIdNotSet,
     /// Database is not mutable
     DatabaseImmutable,
     /// Server not in required state for operation
@@ -444,7 +439,6 @@ pub enum PreconditionViolation {
 impl PreconditionViolation {
     fn description(&self) -> String {
         match self {
-            Self::ServerIdNotSet => "server id must be set".to_string(),
             Self::DatabaseImmutable => "database must be mutable".to_string(),
             Self::ServerInvalidState(description) => description.clone(),
             Self::DatabaseInvalidState(description) => description.clone(),
@@ -460,11 +454,6 @@ impl PreconditionViolation {
 impl From<PreconditionViolation> for rpc::precondition_failure::Violation {
     fn from(v: PreconditionViolation) -> Self {
         match v {
-            PreconditionViolation::ServerIdNotSet => Self {
-                r#type: "server_id".to_string(),
-                subject: "influxdata.com/iox".to_string(),
-                description: v.description(),
-            },
             PreconditionViolation::ServerInvalidState(_) => Self {
                 r#type: "state".to_string(),
                 subject: "influxdata.com/iox".to_string(),
@@ -516,7 +505,6 @@ impl From<PreconditionViolation> for rpc::precondition_failure::Violation {
 impl From<rpc::precondition_failure::Violation> for PreconditionViolation {
     fn from(v: rpc::precondition_failure::Violation) -> Self {
         match (v.r#type.as_str(), v.subject.as_str()) {
-            ("server_id", "influxdata.com/iox") => PreconditionViolation::ServerIdNotSet,
             ("state", "influxdata.com/iox") => {
                 PreconditionViolation::ServerInvalidState(v.description)
             }