Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

default compaction algorithm is configurable via env var in PS, neon_local, test suite #7555

Closed
Tracked by #7554
problame opened this issue Apr 30, 2024 · 3 comments · Fixed by #7747
Closed
Tracked by #7554

default compaction algorithm is configurable via env var in PS, neon_local, test suite #7555

problame opened this issue Apr 30, 2024 · 3 comments · Fixed by #7747
Assignees
@problame
Labels
c/storage/pageserver Component: storage: pageserver

Comments

@problame
Copy link
Contributor

No description provided.

@problame problame mentioned this issue Apr 30, 2024
Open
@problame problame self-assigned this Apr 30, 2024
@problame problame added the c/storage/pageserver Component: storage: pageserver label Apr 30, 2024
problame added a commit that referenced this issue May 3, 2024
@problame
test_runner: remove unused NeonPageserver.config_override field
a258dc3 
refs #7555
This was referenced May 3, 2024
Merged
Merged
problame added a commit that referenced this issue May 3, 2024
@problame
test_runner: remove unused NeonPageserver.config_override field (#7605
ce0ddd7 
)

refs #7555
skyzh added a commit that referenced this issue May 3, 2024
@problame @skyzh
refactor: move NodeMetadata to pageserver_api; use it from `neon_…
1e7cd6a 
…local` (#7606)

This is the first step towards representing all of Pageserver
configuration as clean `serde::Serialize`able Rust structs in
`pageserver_api`.

The `neon_local` code will then use those structs instead of the crude
`toml_edit` / string concatenation that it does today.

refs #7555

---------

Co-authored-by: Alex Chi Z <iskyzh@gmail.com>
@problame problame mentioned this issue May 4, 2024
Merged
@problame
Copy link
Contributor Author

problame commented May 6, 2024

Encountered last week that it's not trivial to override tenant_config.

Went on a bit of a tangent to clean up how the test suite & neon_local deal with the pageserver config.

Resulting patch stack:

(These clean-ups were long overdue anyway.)

Will get these merged this week, then implement this issue on top.

problame added a commit that referenced this issue May 6, 2024
@problame
refactor(pageserver): remove --update-init flag (#7612)
df1def7 
We don't actually use it.

refs #7555
conradludgate pushed a commit that referenced this issue May 8, 2024
@problame @conradludgate
test_runner: remove unused NeonPageserver.config_override field (#7605
a554006 
)

refs #7555
conradludgate pushed a commit that referenced this issue May 8, 2024
@problame @skyzh @conradludgate
refactor: move NodeMetadata to pageserver_api; use it from `neon_…
1ec143d 
…local` (#7606)

This is the first step towards representing all of Pageserver
configuration as clean `serde::Serialize`able Rust structs in
`pageserver_api`.

The `neon_local` code will then use those structs instead of the crude
`toml_edit` / string concatenation that it does today.

refs #7555

---------

Co-authored-by: Alex Chi Z <iskyzh@gmail.com>
conradludgate pushed a commit that referenced this issue May 8, 2024
@problame @conradludgate
refactor(pageserver): remove --update-init flag (#7612)
489254b 
We don't actually use it.

refs #7555
@problame
Copy link
Contributor Author

problame commented May 8, 2024

Had to do more neon_local refactorings, didn't get to completing this issue

On the upside, neon_local is in a much better shape now :)

@problame
Copy link
Contributor Author

This week: actually do this, necessary refactorings have landed.

@problame problame mentioned this issue May 14, 2024
Merged
problame added a commit that referenced this issue May 14, 2024
@problame
test suite: allow overriding default compaction algorithm via env var (…
4eedb3b 
…#7747)

This PR allows setting the
`PAGESERVER_DEFAULT_TENANT_CONFIG_COMPACTION_ALGORITHM` env var to
override the `tenant_config.compaction_algorithm` field in the initial
`pageserver.toml` for all tests.

I tested manually that this works by halting a test using pdb and
inspecting the `effective_config` in the tenant status managment API.

If the env var is set, the tests are parametrized by the `kind` tag
field, allowing to do a matrix build in CI and let Allure summarize
everything in a nice report.

If the env var is not set, the tests are not parametrized. So, merging
this PR doesn't cause problems for flaky test detection. In fact, it
doesn't cause any runtime change if the env var is not set.

There are some tests in the test suite that set used to override
the entire tenant_config using
`NeonEnvBuilder.pageserver_config_override`.
Since config overrides are merged non-recursively, such overrides
that don't specify `kind = ` cause a fallback to pageserver's built-in
`DEFAULT_COMPACTION_ALGORITHM`.

Such cases can be found using

```
["']tenant_config\s*[='"]
```

We'll deal with these tests in a future PR.

closes #7555
a-masterov pushed a commit that referenced this issue May 20, 2024
@problame @a-masterov
test suite: allow overriding default compaction algorithm via env var (…
b5ae9e1 
…#7747)

This PR allows setting the
`PAGESERVER_DEFAULT_TENANT_CONFIG_COMPACTION_ALGORITHM` env var to
override the `tenant_config.compaction_algorithm` field in the initial
`pageserver.toml` for all tests.

I tested manually that this works by halting a test using pdb and
inspecting the `effective_config` in the tenant status managment API.

If the env var is set, the tests are parametrized by the `kind` tag
field, allowing to do a matrix build in CI and let Allure summarize
everything in a nice report.

If the env var is not set, the tests are not parametrized. So, merging
this PR doesn't cause problems for flaky test detection. In fact, it
doesn't cause any runtime change if the env var is not set.

There are some tests in the test suite that set used to override
the entire tenant_config using
`NeonEnvBuilder.pageserver_config_override`.
Since config overrides are merged non-recursively, such overrides
that don't specify `kind = ` cause a fallback to pageserver's built-in
`DEFAULT_COMPACTION_ALGORITHM`.

Such cases can be found using

```
["']tenant_config\s*[='"]
```

We'll deal with these tests in a future PR.

closes #7555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@problame problame

Labels
c/storage/pageserver Component: storage: pageserver
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test suite: allow overriding default compaction algorithm via env var neondatabase/neon
1 participant
@problame