Skip to content

Commit 31eff7a

Browse files
Itay-Tsabary-StarkwareItay-Tsabary-Starkware
committed
apollo_deployments: further cleanups
1 parent 1ff29f4 commit 31eff7a

File tree

4 files changed

+91
-124
lines changed

4 files changed

+91
-124
lines changed

‎crates/apollo_deployments/src/deployment_definitions.rs‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
use apollo_node_config::component_execution_config::DEFAULT_INVALID_PORT;
21
use serde::Serialize;
3-
use static_assertions::const_assert_ne;
42
use strum::EnumIter;
53
use strum_macros::{AsRefStr, Display};
64

@@ -9,34 +7,10 @@ use strum_macros::{AsRefStr, Display};
97
mod deployment_definitions_test;
108

119
pub(crate) const CONFIG_BASE_DIR: &str = "crates/apollo_deployments/resources/";
12-
pub(crate) const INFRA_PORT_PLACEHOLDER: u16 = 1;
13-
const_assert_ne!(INFRA_PORT_PLACEHOLDER, DEFAULT_INVALID_PORT);
10+
pub(crate) const RETRIES_FOR_L1_SERVICES: usize = 0;
1411

1512
const BASE_APP_CONFIGS_DIR_PATH: &str = "crates/apollo_deployments/resources/app_configs";
1613

17-
// TODO(Nadin): Integrate this logic with `ComponentConfigInService` once the merge from main-14.0
18-
// is complete.
19-
20-
#[derive(Clone, Copy, Debug, EnumIter, Display, Serialize, Ord, PartialEq, Eq, PartialOrd)]
21-
pub enum InfraServicePort {
22-
Batcher,
23-
ClassManager,
24-
Committer,
25-
Gateway,
26-
L1GasPriceProvider,
27-
L1Provider,
28-
Mempool,
29-
SierraCompiler,
30-
SignatureManager,
31-
StateSync,
32-
}
33-
34-
impl InfraServicePort {
35-
pub fn get_port(&self) -> u16 {
36-
INFRA_PORT_PLACEHOLDER
37-
}
38-
}
39-
4014
#[derive(
4115
Hash, Clone, Debug, Display, Serialize, PartialEq, Eq, PartialOrd, Ord, EnumIter, AsRefStr,
4216
)]

‎crates/apollo_deployments/src/deployments/distributed.rs‎

Lines changed: 18 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{BTreeMap, BTreeSet, HashMap};
1+
use std::collections::{BTreeSet, HashMap};
22

33
use apollo_infra::component_client::DEFAULT_RETRIES;
44
use apollo_node_config::component_config::ComponentConfig;
@@ -10,19 +10,14 @@ use serde::Serialize;
1010
use strum::{Display, IntoEnumIterator};
1111
use strum_macros::{AsRefStr, EnumIter};
1212

13-
use crate::deployment_definitions::{
14-
ComponentConfigInService,
15-
InfraServicePort,
16-
INFRA_PORT_PLACEHOLDER,
17-
};
13+
use crate::deployment_definitions::{ComponentConfigInService, RETRIES_FOR_L1_SERVICES};
1814
use crate::scale_policy::ScalePolicy;
1915
use crate::service::{GetComponentConfigs, NodeService, ServiceNameInner};
20-
use crate::utils::validate_ports;
16+
use crate::utils::InfraPortAllocator;
2117

18+
// Number of infra-required ports for a distributed node service distribution.
2219
pub const DISTRIBUTED_NODE_REQUIRED_PORTS_NUM: usize = 10;
2320

24-
pub const RETRIES_FOR_L1_SERVICES: usize = 0;
25-
2621
// TODO(Tsabary): define consts and functions whenever relevant.
2722

2823
#[derive(Clone, Copy, Debug, Display, PartialEq, Eq, Hash, Serialize, AsRefStr, EnumIter)]
@@ -50,45 +45,20 @@ impl From<DistributedNodeServiceName> for NodeService {
5045

5146
impl GetComponentConfigs for DistributedNodeServiceName {
5247
fn get_component_configs(ports: Option<Vec<u16>>) -> HashMap<NodeService, ComponentConfig> {
53-
// TODO(Tsabary): style this code, i.e., no need to use a mutable map nor the for loop, and
54-
// can simply collect the required values.
55-
let mut service_ports: BTreeMap<InfraServicePort, u16> = BTreeMap::new();
56-
match ports {
57-
Some(ports) => {
58-
// TODO(Nadin): This should compare against DistributedServicePort-specific infra
59-
// ports, not all InfraServicePort variants.
60-
validate_ports(&ports, InfraServicePort::iter().count());
61-
for (service_port, port) in InfraServicePort::iter().zip(ports) {
62-
service_ports.insert(service_port, port);
63-
}
64-
}
65-
None => {
66-
for service_port in InfraServicePort::iter() {
67-
service_ports.insert(service_port, INFRA_PORT_PLACEHOLDER);
68-
}
69-
}
70-
};
71-
72-
let batcher =
73-
Self::Batcher.component_config_pair(service_ports[&InfraServicePort::Batcher]);
74-
let class_manager = Self::ClassManager
75-
.component_config_pair(service_ports[&InfraServicePort::ClassManager]);
76-
let committer =
77-
Self::Committer.component_config_pair(service_ports[&InfraServicePort::Committer]);
78-
let gateway =
79-
Self::Gateway.component_config_pair(service_ports[&InfraServicePort::Gateway]);
80-
let l1_gas_price_provider =
81-
Self::L1.component_config_pair(service_ports[&InfraServicePort::L1GasPriceProvider]);
82-
let l1_provider =
83-
Self::L1.component_config_pair(service_ports[&InfraServicePort::L1Provider]);
84-
let mempool =
85-
Self::Mempool.component_config_pair(service_ports[&InfraServicePort::Mempool]);
86-
let sierra_compiler = Self::SierraCompiler
87-
.component_config_pair(service_ports[&InfraServicePort::SierraCompiler]);
88-
let signature_manager = Self::SignatureManager
89-
.component_config_pair(service_ports[&InfraServicePort::SignatureManager]);
90-
let state_sync =
91-
Self::StateSync.component_config_pair(service_ports[&InfraServicePort::StateSync]);
48+
let mut infra_port_allocator =
49+
InfraPortAllocator::new(ports, DISTRIBUTED_NODE_REQUIRED_PORTS_NUM);
50+
let batcher = Self::Batcher.component_config_pair(infra_port_allocator.next());
51+
let class_manager = Self::ClassManager.component_config_pair(infra_port_allocator.next());
52+
let committer = Self::Committer.component_config_pair(infra_port_allocator.next());
53+
let gateway = Self::Gateway.component_config_pair(infra_port_allocator.next());
54+
let l1_gas_price_provider = Self::L1.component_config_pair(infra_port_allocator.next());
55+
let l1_provider = Self::L1.component_config_pair(infra_port_allocator.next());
56+
let mempool = Self::Mempool.component_config_pair(infra_port_allocator.next());
57+
let sierra_compiler =
58+
Self::SierraCompiler.component_config_pair(infra_port_allocator.next());
59+
let signature_manager =
60+
Self::SignatureManager.component_config_pair(infra_port_allocator.next());
61+
let state_sync = Self::StateSync.component_config_pair(infra_port_allocator.next());
9262

9363
let mut component_config_map = HashMap::<NodeService, ComponentConfig>::new();
9464
for inner_service_name in Self::iter() {

‎crates/apollo_deployments/src/deployments/hybrid.rs‎

Lines changed: 17 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{BTreeMap, BTreeSet, HashMap};
1+
use std::collections::{BTreeSet, HashMap};
22

33
use apollo_infra::component_client::DEFAULT_RETRIES;
44
use apollo_node_config::component_config::ComponentConfig;
@@ -10,16 +10,12 @@ use serde::Serialize;
1010
use strum::{Display, IntoEnumIterator};
1111
use strum_macros::{AsRefStr, EnumIter};
1212

13-
use crate::deployment_definitions::{
14-
ComponentConfigInService,
15-
InfraServicePort,
16-
INFRA_PORT_PLACEHOLDER,
17-
};
18-
use crate::deployments::distributed::RETRIES_FOR_L1_SERVICES;
13+
use crate::deployment_definitions::{ComponentConfigInService, RETRIES_FOR_L1_SERVICES};
1914
use crate::scale_policy::ScalePolicy;
2015
use crate::service::{GetComponentConfigs, NodeService, ServiceNameInner};
21-
use crate::utils::validate_ports;
16+
use crate::utils::InfraPortAllocator;
2217

18+
// Number of infra-required ports for a hybrid node service distribution.
2319
pub const HYBRID_NODE_REQUIRED_PORTS_NUM: usize = 10;
2420

2521
#[derive(Clone, Copy, Debug, Display, PartialEq, Eq, Hash, Serialize, AsRefStr, EnumIter)]
@@ -42,45 +38,19 @@ impl From<HybridNodeServiceName> for NodeService {
4238

4339
impl GetComponentConfigs for HybridNodeServiceName {
4440
fn get_component_configs(ports: Option<Vec<u16>>) -> HashMap<NodeService, ComponentConfig> {
45-
// TODO(Tsabary): style this code, i.e., no need to use a mutable map nor the for loop, and
46-
// can simply collect the required values.
47-
let mut service_ports: BTreeMap<InfraServicePort, u16> = BTreeMap::new();
48-
match ports {
49-
Some(ports) => {
50-
validate_ports(&ports, InfraServicePort::iter().count());
51-
// TODO(Nadin): This should compare against HybridServicePort-specific infra ports,
52-
// not all InfraServicePort variants.
53-
for (service_port, port) in InfraServicePort::iter().zip(ports) {
54-
service_ports.insert(service_port, port);
55-
}
56-
}
57-
None => {
58-
// Extract the infra service ports for all inner services of the hybrid node.
59-
for service_port in InfraServicePort::iter() {
60-
service_ports.insert(service_port, INFRA_PORT_PLACEHOLDER);
61-
}
62-
}
63-
};
64-
65-
let batcher = Self::Core.component_config_pair(service_ports[&InfraServicePort::Batcher]);
66-
let class_manager =
67-
Self::Core.component_config_pair(service_ports[&InfraServicePort::ClassManager]);
68-
let committer =
69-
Self::Committer.component_config_pair(service_ports[&InfraServicePort::Committer]);
70-
let gateway =
71-
Self::Gateway.component_config_pair(service_ports[&InfraServicePort::Gateway]);
72-
let l1_gas_price_provider =
73-
Self::L1.component_config_pair(service_ports[&InfraServicePort::L1GasPriceProvider]);
74-
let l1_provider =
75-
Self::L1.component_config_pair(service_ports[&InfraServicePort::L1Provider]);
76-
let mempool =
77-
Self::Mempool.component_config_pair(service_ports[&InfraServicePort::Mempool]);
78-
let sierra_compiler = Self::SierraCompiler
79-
.component_config_pair(service_ports[&InfraServicePort::SierraCompiler]);
80-
let signature_manager =
81-
Self::Core.component_config_pair(service_ports[&InfraServicePort::SignatureManager]);
82-
let state_sync =
83-
Self::Core.component_config_pair(service_ports[&InfraServicePort::StateSync]);
41+
let mut infra_port_allocator =
42+
InfraPortAllocator::new(ports, HYBRID_NODE_REQUIRED_PORTS_NUM);
43+
let batcher = Self::Core.component_config_pair(infra_port_allocator.next());
44+
let class_manager = Self::Core.component_config_pair(infra_port_allocator.next());
45+
let committer = Self::Committer.component_config_pair(infra_port_allocator.next());
46+
let gateway = Self::Gateway.component_config_pair(infra_port_allocator.next());
47+
let l1_gas_price_provider = Self::L1.component_config_pair(infra_port_allocator.next());
48+
let l1_provider = Self::L1.component_config_pair(infra_port_allocator.next());
49+
let mempool = Self::Mempool.component_config_pair(infra_port_allocator.next());
50+
let sierra_compiler =
51+
Self::SierraCompiler.component_config_pair(infra_port_allocator.next());
52+
let signature_manager = Self::Core.component_config_pair(infra_port_allocator.next());
53+
let state_sync = Self::Core.component_config_pair(infra_port_allocator.next());
8454

8555
let mut component_config_map = HashMap::<NodeService, ComponentConfig>::new();
8656
for inner_service_name in Self::iter() {

‎crates/apollo_deployments/src/utils.rs‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,60 @@
11
use std::collections::HashSet;
22

3-
/// Validates that the provided ports vector has the correct length and all unique values.
4-
pub(crate) fn validate_ports(ports: &[u16], required_ports_num: usize) {
3+
use apollo_node_config::component_execution_config::DEFAULT_INVALID_PORT;
4+
use static_assertions::const_assert_ne;
5+
6+
const INFRA_PORT_PLACEHOLDER: u16 = 1;
7+
const_assert_ne!(INFRA_PORT_PLACEHOLDER, DEFAULT_INVALID_PORT);
8+
9+
/// A generator-like for setting infra ports in different services.
10+
/// - If constructed with `Some(vec)`: yields values from the vec, up to `expected_len` times.
11+
/// - If constructed with `None`: yields `INFRA_PORT_PLACEHOLDER`, up to `expected_len` times.
12+
/// - On `Drop`: asserts it has been fully depleted (all `expected_len` values were yielded).
13+
pub(crate) struct InfraPortAllocator {
14+
expected_len: usize,
15+
idx: usize,
16+
values: Vec<u16>,
17+
}
18+
19+
impl InfraPortAllocator {
20+
pub fn new(values: Option<Vec<u16>>, expected_len: usize) -> Self {
21+
let values = match values {
22+
Some(v) => {
23+
validate_ports(&v, expected_len);
24+
v
25+
}
26+
None => vec![INFRA_PORT_PLACEHOLDER; expected_len],
27+
};
28+
29+
Self { expected_len, idx: 0, values }
30+
}
31+
32+
/// Returns the next value. Panics if called more than `expected_len` times.
33+
pub fn next(&mut self) -> u16 {
34+
assert!(
35+
self.idx < self.expected_len,
36+
"InfraPortAllocator exhausted: expected_len is {}",
37+
self.expected_len
38+
);
39+
let out = self.values[self.idx];
40+
self.idx += 1;
41+
out
42+
}
43+
}
44+
45+
impl Drop for InfraPortAllocator {
46+
fn drop(&mut self) {
47+
assert!(
48+
self.idx == self.expected_len,
49+
"InfraPortAllocator dropped before being depleted: produced {} out of {} values",
50+
self.idx,
51+
self.expected_len
52+
);
53+
}
54+
}
55+
56+
// Validates that the provided ports vector has the correct length and all unique values.
57+
fn validate_ports(ports: &[u16], required_ports_num: usize) {
558
let ports_len = ports.len();
659
assert_eq!(
760
ports_len, required_ports_num,

0 commit comments

Comments
 (0)