Skip to content

Commit

Permalink
chore(pageserver): use kebab case for aux file flag (#7840)
Browse files Browse the repository at this point in the history
part of #7462

---------

Signed-off-by: Alex Chi Z <chi@neon.tech>
  • Loading branch information
skyzh committed May 22, 2024
1 parent 9cfe08e commit ddd8ebd
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 28 deletions.
45 changes: 26 additions & 19 deletions libs/pageserver_api/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::{
collections::HashMap,
io::{BufRead, Read},
num::{NonZeroU64, NonZeroUsize},
str::FromStr,
sync::atomic::AtomicUsize,
time::{Duration, SystemTime},
};
Expand Down Expand Up @@ -334,14 +333,28 @@ pub struct TenantConfig {
/// Unset -> V1
/// -> V2
/// -> CrossValidation -> V2
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(
Eq,
PartialEq,
Debug,
Copy,
Clone,
strum_macros::EnumString,
strum_macros::Display,
serde_with::DeserializeFromStr,
serde_with::SerializeDisplay,
)]
#[strum(serialize_all = "kebab-case")]
pub enum AuxFilePolicy {
/// V1 aux file policy: store everything in AUX_FILE_KEY
#[strum(ascii_case_insensitive)]
V1,
/// V2 aux file policy: store in the AUX_FILE keyspace
#[strum(ascii_case_insensitive)]
V2,
/// Cross validation runs both formats on the write path and does validation
/// on the read path.
#[strum(ascii_case_insensitive)]
CrossValidation,
}

Expand Down Expand Up @@ -407,23 +420,6 @@ impl AuxFilePolicy {
}
}

impl FromStr for AuxFilePolicy {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.to_lowercase();
if s == "v1" {
Ok(Self::V1)
} else if s == "v2" {
Ok(Self::V2)
} else if s == "crossvalidation" || s == "cross_validation" {
Ok(Self::CrossValidation)
} else {
anyhow::bail!("cannot parse {} to aux file policy", s)
}
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "kind")]
pub enum EvictionPolicy {
Expand Down Expand Up @@ -1405,6 +1401,7 @@ impl PagestreamBeMessage {
#[cfg(test)]
mod tests {
use serde_json::json;
use std::str::FromStr;

use super::*;

Expand Down Expand Up @@ -1667,4 +1664,14 @@ mod tests {
AuxFilePolicy::V2
));
}

#[test]
fn test_aux_parse() {
assert_eq!(AuxFilePolicy::from_str("V2").unwrap(), AuxFilePolicy::V2);
assert_eq!(AuxFilePolicy::from_str("v2").unwrap(), AuxFilePolicy::V2);
assert_eq!(
AuxFilePolicy::from_str("cross-validation").unwrap(),
AuxFilePolicy::CrossValidation
);
}
}
2 changes: 1 addition & 1 deletion test_runner/fixtures/neon_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,7 @@ def create_tenant(
args.extend(["-c", "switch_aux_file_policy:v1"])

if aux_file_v2 is AuxFileStore.CrossValidation:
args.extend(["-c", "switch_aux_file_policy:cross_validation"])
args.extend(["-c", "switch_aux_file_policy:cross-validation"])

if set_default:
args.append("--set-default")
Expand Down
6 changes: 3 additions & 3 deletions test_runner/fixtures/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,9 @@ def assert_no_errors(log_file, service, allowed_errors):

@enum.unique
class AuxFileStore(str, enum.Enum):
V1 = "V1"
V2 = "V2"
CrossValidation = "CrossValidation"
V1 = "v1"
V2 = "v2"
CrossValidation = "cross-validation"

def __repr__(self) -> str:
return f"'aux-{self.value}'"
Expand Down
2 changes: 1 addition & 1 deletion test_runner/regress/test_attach_tenant_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def test_fully_custom_config(positive_env: NeonEnv):
"trace_read_requests": True,
"walreceiver_connect_timeout": "13m",
"image_layer_creation_check_threshold": 1,
"switch_aux_file_policy": "CrossValidation",
"switch_aux_file_policy": "cross-validation",
}

ps_http = env.pageserver.http_client()
Expand Down
12 changes: 8 additions & 4 deletions test_runner/regress/test_aux_files.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from fixtures.log_helper import log
from fixtures.neon_fixtures import (
AuxFileStore,
NeonEnvBuilder,
logical_replication_sync,
)
Expand All @@ -14,7 +15,7 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg):
timeline_id = env.initial_timeline

tenant_config = client.tenant_config(tenant_id).effective_config
tenant_config["switch_aux_file_policy"] = "V2"
tenant_config["switch_aux_file_policy"] = AuxFileStore.V2
client.set_tenant_config(tenant_id, tenant_config)
# aux file v2 is enabled on the write path, so for now, it should be unset (or null)
assert (
Expand Down Expand Up @@ -49,7 +50,10 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg):

with env.pageserver.http_client() as client:
# aux file v2 flag should be enabled at this point
assert client.timeline_detail(tenant_id, timeline_id)["last_aux_file_policy"] == "V2"
assert (
client.timeline_detail(tenant_id, timeline_id)["last_aux_file_policy"]
== AuxFileStore.V2
)
with env.pageserver.http_client() as client:
tenant_config = client.tenant_config(tenant_id).effective_config
tenant_config["switch_aux_file_policy"] = "V1"
Expand All @@ -59,7 +63,7 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg):
client.timeline_detail(tenant_id=tenant_id, timeline_id=timeline_id)[
"last_aux_file_policy"
]
== "V2"
== AuxFileStore.V2
)
env.pageserver.restart()
with env.pageserver.http_client() as client:
Expand All @@ -68,5 +72,5 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg):
client.timeline_detail(tenant_id=tenant_id, timeline_id=timeline_id)[
"last_aux_file_policy"
]
== "V2"
== AuxFileStore.V2
)

1 comment on commit ddd8ebd

@github-actions
Copy link

Choose a reason for hiding this comment

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

3196 tests run: 3055 passed, 1 failed, 140 skipped (full report)


Failures on Postgres 14

  • test_basebackup_with_high_slru_count[github-actions-selfhosted-sequential-10-13-30]: release
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_basebackup_with_high_slru_count[release-pg14-github-actions-selfhosted-sequential-10-13-30]"
Flaky tests (1)

Postgres 15

  • test_pageserver_restarts_under_worload: debug

Code coverage* (full report)

  • functions: 31.5% (6447 of 20498 functions)
  • lines: 48.3% (49795 of 103167 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
ddd8ebd at 2024-05-22T18:33:52.793Z :recycle:

Please sign in to comment.