Skip to content

Commit

Permalink
fix(config)!: fix concurrency default & docs (#18651)
Browse files Browse the repository at this point in the history
* fix(config): fix concurrency default & docs

* fix wording

* fix test

* remove examples

* update cue

* feedback

* upgrade guide

* nit
  • Loading branch information
dsmith3197 authored Sep 26, 2023
1 parent 483096c commit 26ced36
Show file tree
Hide file tree
Showing 45 changed files with 369 additions and 141 deletions.
8 changes: 4 additions & 4 deletions src/sinks/util/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use tower::{
use vector_config::configurable_component;

pub use crate::sinks::util::service::{
concurrency::{concurrency_is_none, Concurrency},
concurrency::{concurrency_is_adaptive, Concurrency},
health::{HealthConfig, HealthLogic, HealthService},
map::Map,
};
Expand Down Expand Up @@ -93,7 +93,7 @@ impl<L> ServiceBuilderExt<L> for ServiceBuilder<L> {
pub struct TowerRequestConfig {
#[configurable(derived)]
#[serde(default = "default_concurrency")]
#[serde(skip_serializing_if = "concurrency_is_none")]
#[serde(skip_serializing_if = "concurrency_is_adaptive")]
pub concurrency: Concurrency,

/// The time a request can take before being aborted.
Expand Down Expand Up @@ -144,7 +144,7 @@ pub struct TowerRequestConfig {
}

const fn default_concurrency() -> Concurrency {
Concurrency::None
Concurrency::Adaptive
}

const fn default_timeout_secs() -> Option<u64> {
Expand Down Expand Up @@ -457,7 +457,7 @@ mod tests {
toml::from_str::<TowerRequestConfig>(&toml).expect("Default config failed");

let cfg = toml::from_str::<TowerRequestConfig>("").expect("Empty config failed");
assert_eq!(cfg.concurrency, Concurrency::None);
assert_eq!(cfg.concurrency, Concurrency::Adaptive);

let cfg = toml::from_str::<TowerRequestConfig>("concurrency = 10")
.expect("Fixed concurrency failed");
Expand Down
29 changes: 19 additions & 10 deletions src/sinks/util/service/concurrency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ use serde::{
};

/// Configuration for outbound request concurrency.
///
/// This can be set either to one of the below enum values or to a positive integer, which denotes
/// a fixed concurrency limit.
#[derive(Clone, Copy, Debug, Derivative, Eq, PartialEq)]
pub enum Concurrency {
/// A fixed concurrency of 1.
Expand Down Expand Up @@ -48,28 +51,29 @@ impl Serialize for Concurrency {

impl Default for Concurrency {
fn default() -> Self {
Self::None
Self::Adaptive
}
}

impl Concurrency {
pub const fn if_none(self, other: Self) -> Self {
const fn if_adaptive(self, other: Self) -> Self {
match self {
Self::None => other,
Self::Adaptive => other,
_ => self,
}
}

pub const fn parse_concurrency(&self, default: Self) -> Option<usize> {
match self.if_none(default) {
Concurrency::None | Concurrency::Adaptive => None,
pub const fn parse_concurrency(&self, other: Self) -> Option<usize> {
match self.if_adaptive(other) {
Concurrency::None => Some(1),
Concurrency::Adaptive => None,
Concurrency::Fixed(limit) => Some(limit),
}
}
}

pub const fn concurrency_is_none(concurrency: &Concurrency) -> bool {
matches!(concurrency, Concurrency::None)
pub const fn concurrency_is_adaptive(concurrency: &Concurrency) -> bool {
matches!(concurrency, Concurrency::Adaptive)
}

impl<'de> Deserialize<'de> for Concurrency {
Expand All @@ -93,7 +97,7 @@ impl<'de> Deserialize<'de> for Concurrency {
} else if value == "none" {
Ok(Concurrency::None)
} else {
Err(de::Error::unknown_variant(value, &["adaptive"]))
Err(de::Error::unknown_variant(value, &["adaptive", "none"]))
}
}

Expand Down Expand Up @@ -132,7 +136,12 @@ impl Configurable for Concurrency {

fn metadata() -> Metadata {
let mut metadata = Metadata::default();
metadata.set_description("Configuration for outbound request concurrency.");
metadata.set_description(
r#"Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit."#,
);
metadata.add_custom_attribute(CustomAttribute::kv("docs::enum_tagging", "external"));
metadata
}
Expand Down
11 changes: 10 additions & 1 deletion website/content/en/highlights/2023-09-26-0-33-0-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ and **deprecations**:

1. [Default config location change](#default-config-location-change)
1. [Renaming the `armv7` rpm package](#armv7-rename)
2. [Metadata field in the Vector protobuf definition](#vector-proto-metadata)
1. [Metadata field in the Vector protobuf definition](#vector-proto-metadata)

and **potentially impactful changes**:

1. [Async runtime default number of worker threads](#runtime-worker-threads)
1. [Setting `request.concurrency = "none"`](#request-concurrency)

We cover them below to help you upgrade quickly:

Expand Down Expand Up @@ -91,3 +92,11 @@ has limited quotas, but note this change may impact performance.

The number of worker threads used can be seen by enabling debug logging, and the value can
be overriden by setting the `VECTOR_THREADS` environment variable.

#### Setting `request.concurrency = "none"` {#request-concurrency}

Explicitly setting `request.concurrency = "none"` in a sink configuration now properly configures
the sink to have a fixed concurrency limit of 1, as was stated in the documentation. Previously, the
above configuration would result in the sink using adaptive request concurrency. The default setting
remains unchanged (adaptive request concurrency).

11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/appsignal.cue
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,16 @@ base: components: sinks: appsignal: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,11 +509,16 @@ base: components: sinks: aws_cloudwatch_logs: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,16 @@ base: components: sinks: aws_cloudwatch_metrics: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,11 +480,16 @@ base: components: sinks: aws_kinesis_firehose: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,16 @@ base: components: sinks: aws_kinesis_streams: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/aws_s3.cue
Original file line number Diff line number Diff line change
Expand Up @@ -726,11 +726,16 @@ base: components: sinks: aws_s3: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/aws_sns.cue
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,16 @@ base: components: sinks: aws_sns: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/aws_sqs.cue
Original file line number Diff line number Diff line change
Expand Up @@ -442,11 +442,16 @@ base: components: sinks: aws_sqs: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/axiom.cue
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,16 @@ base: components: sinks: axiom: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/azure_blob.cue
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,16 @@ base: components: sinks: azure_blob: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,16 @@ base: components: sinks: azure_monitor_logs: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/clickhouse.cue
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,16 @@ base: components: sinks: clickhouse: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
11 changes: 8 additions & 3 deletions website/cue/reference/components/sinks/base/databend.cue
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,16 @@ base: components: sinks: databend: configuration: {
}
}
concurrency: {
description: "Configuration for outbound request concurrency."
required: false
description: """
Configuration for outbound request concurrency.
This can be set either to one of the below enum values or to a positive integer, which denotes
a fixed concurrency limit.
"""
required: false
type: {
string: {
default: "none"
default: "adaptive"
enum: {
adaptive: """
Concurrency will be managed by Vector's [Adaptive Request Concurrency][arc] feature.
Expand Down
Loading

0 comments on commit 26ced36

Please sign in to comment.