From cdaa2816e7464c2517fee2f56f65b9b401ea946f Mon Sep 17 00:00:00 2001 From: Vlad Lazar Date: Fri, 26 Jul 2024 14:19:52 +0100 Subject: [PATCH] pageserver: make vectored get the default read path for the pageserver (#8384) ## Problem Vectored get is already enabled in all prod regions without validation. The pageserver defaults are out of sync however. ## Summary of changes Update the pageserver defaults to match the prod config. Also means that when running tests locally, people don't have to use the env vars to get the prod config. --- .github/workflows/_build-and-test-locally.yml | 3 --- .github/workflows/build_and_test.yml | 3 --- pageserver/src/config.rs | 6 +++--- test_runner/fixtures/neon_fixtures.py | 21 ------------------- test_runner/regress/test_broken_timeline.py | 9 ++------ test_runner/regress/test_compatibility.py | 6 ------ 6 files changed, 5 insertions(+), 43 deletions(-) diff --git a/.github/workflows/_build-and-test-locally.yml b/.github/workflows/_build-and-test-locally.yml index 35c6251304..26e234a04d 100644 --- a/.github/workflows/_build-and-test-locally.yml +++ b/.github/workflows/_build-and-test-locally.yml @@ -278,9 +278,6 @@ jobs: CHECK_ONDISK_DATA_COMPATIBILITY: nonempty BUILD_TAG: ${{ inputs.build-tag }} PAGESERVER_VIRTUAL_FILE_IO_ENGINE: tokio-epoll-uring - PAGESERVER_GET_VECTORED_IMPL: vectored - PAGESERVER_GET_IMPL: vectored - PAGESERVER_VALIDATE_VEC_GET: true # Temporary disable this step until we figure out why it's so flaky # Ref https://github.com/neondatabase/neon/issues/4540 diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 885f4058d0..872c1fbb39 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -286,9 +286,6 @@ jobs: PERF_TEST_RESULT_CONNSTR: "${{ secrets.PERF_TEST_RESULT_CONNSTR }}" TEST_RESULT_CONNSTR: "${{ secrets.REGRESS_TEST_RESULT_CONNSTR_NEW }}" PAGESERVER_VIRTUAL_FILE_IO_ENGINE: tokio-epoll-uring - PAGESERVER_GET_VECTORED_IMPL: vectored - PAGESERVER_GET_IMPL: vectored - PAGESERVER_VALIDATE_VEC_GET: false # XXX: no coverage data handling here, since benchmarks are run on release builds, # while coverage is currently collected for the debug ones diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 20e78b1d85..614bbf3392 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -83,16 +83,16 @@ pub mod defaults { #[cfg(not(target_os = "linux"))] pub const DEFAULT_VIRTUAL_FILE_IO_ENGINE: &str = "std-fs"; - pub const DEFAULT_GET_VECTORED_IMPL: &str = "sequential"; + pub const DEFAULT_GET_VECTORED_IMPL: &str = "vectored"; - pub const DEFAULT_GET_IMPL: &str = "legacy"; + pub const DEFAULT_GET_IMPL: &str = "vectored"; pub const DEFAULT_MAX_VECTORED_READ_BYTES: usize = 128 * 1024; // 128 KiB pub const DEFAULT_IMAGE_COMPRESSION: ImageCompressionAlgorithm = ImageCompressionAlgorithm::Disabled; - pub const DEFAULT_VALIDATE_VECTORED_GET: bool = true; + pub const DEFAULT_VALIDATE_VECTORED_GET: bool = false; pub const DEFAULT_EPHEMERAL_BYTES_PER_MEMORY_KB: usize = 0; diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index d6718fca39..09c28148b4 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -542,21 +542,6 @@ class NeonEnvBuilder: f"Overriding pageserver default compaction algorithm to {self.pageserver_default_tenant_config_compaction_algorithm}" ) - self.pageserver_get_vectored_impl: Optional[str] = None - if os.getenv("PAGESERVER_GET_VECTORED_IMPL", "") == "vectored": - self.pageserver_get_vectored_impl = "vectored" - log.debug('Overriding pageserver get_vectored_impl config to "vectored"') - - self.pageserver_get_impl: Optional[str] = None - if os.getenv("PAGESERVER_GET_IMPL", "") == "vectored": - self.pageserver_get_impl = "vectored" - log.debug('Overriding pageserver get_impl config to "vectored"') - - self.pageserver_validate_vectored_get: Optional[bool] = None - if (validate := os.getenv("PAGESERVER_VALIDATE_VEC_GET")) is not None: - self.pageserver_validate_vectored_get = bool(validate) - log.debug(f'Overriding pageserver validate_vectored_get config to "{validate}"') - self.pageserver_aux_file_policy = pageserver_aux_file_policy self.safekeeper_extra_opts = safekeeper_extra_opts @@ -1157,12 +1142,6 @@ class NeonEnv: } if self.pageserver_virtual_file_io_engine is not None: ps_cfg["virtual_file_io_engine"] = self.pageserver_virtual_file_io_engine - if config.pageserver_get_vectored_impl is not None: - ps_cfg["get_vectored_impl"] = config.pageserver_get_vectored_impl - if config.pageserver_get_impl is not None: - ps_cfg["get_impl"] = config.pageserver_get_impl - if config.pageserver_validate_vectored_get is not None: - ps_cfg["validate_vectored_get"] = config.pageserver_validate_vectored_get if config.pageserver_default_tenant_config_compaction_algorithm is not None: tenant_config = ps_cfg.setdefault("tenant_config", {}) tenant_config[ diff --git a/test_runner/regress/test_broken_timeline.py b/test_runner/regress/test_broken_timeline.py index 61afd820ca..976ac09335 100644 --- a/test_runner/regress/test_broken_timeline.py +++ b/test_runner/regress/test_broken_timeline.py @@ -17,16 +17,11 @@ from fixtures.pg_version import PgVersion # Test restarting page server, while safekeeper and compute node keep # running. def test_local_corruption(neon_env_builder: NeonEnvBuilder): - if neon_env_builder.pageserver_get_impl == "vectored": - reconstruct_function_name = "get_values_reconstruct_data" - else: - reconstruct_function_name = "get_value_reconstruct_data" - env = neon_env_builder.init_start() env.pageserver.allowed_errors.extend( [ - f".*{reconstruct_function_name} for layer .*", + ".*get_values_reconstruct_data for layer .*", ".*could not find data for key.*", ".*is not active. Current state: Broken.*", ".*will not become active. Current state: Broken.*", @@ -79,7 +74,7 @@ def test_local_corruption(neon_env_builder: NeonEnvBuilder): # (We don't check layer file contents on startup, when loading the timeline) # # This will change when we implement checksums for layers - with pytest.raises(Exception, match=f"{reconstruct_function_name} for layer ") as err: + with pytest.raises(Exception, match="get_values_reconstruct_data for layer ") as err: pg1.start() log.info( f"As expected, compute startup failed for timeline {tenant1}/{timeline1} with corrupt layers: {err}" diff --git a/test_runner/regress/test_compatibility.py b/test_runner/regress/test_compatibility.py index 65649e0c0a..411b20b2c4 100644 --- a/test_runner/regress/test_compatibility.py +++ b/test_runner/regress/test_compatibility.py @@ -227,12 +227,6 @@ def test_forward_compatibility( ) try: - # Previous version neon_local and pageserver are not aware - # of the new config. - # TODO: remove these once the previous version of neon local supports them - neon_env_builder.pageserver_get_impl = None - neon_env_builder.pageserver_validate_vectored_get = None - neon_env_builder.num_safekeepers = 3 # Use previous version's production binaries (pageserver, safekeeper, pg_distrib_dir, etc.).