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.

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

ongoing: fix: consumption metrics on restart #5297

Closed
wants to merge 46 commits into from

Conversation

koivunej
Copy link
Member

@koivunej koivunej commented Sep 13, 2023

Problem

Fixes: #5206
Fixes: #5175
Split off: #5298, #5315, #5316, #5317, #5323, #5324, #5325, #5326, #5327

Summary of changes

(earlier have been split off)

  • split largeish test files
  • split huge modules

@koivunej koivunej force-pushed the refactor_consumption_metrics_test branch from 24d8eef to 4babbc4 Compare September 13, 2023 15:17
koivunej added a commit that referenced this pull request Sep 13, 2023
Refactor tests to have less globals.

This will allow to hopefully write more complex tests for our new metric
collection requirements in #5297. Includes reverted work from #4761
related to test globals.

Co-authored-by: Alexander Bayandin <[email protected]>
Co-authored-by: MMeent <[email protected]>
@koivunej koivunej force-pushed the refactor_consumption_metrics_test branch 2 times, most recently from 22ec922 to 45c76d2 Compare September 14, 2023 15:45
@github-actions
Copy link

github-actions bot commented Sep 14, 2023

2472 tests run: 2353 passed, 1 failed, 118 skipped (full report)


Failures on Postgres 14

  • test_get_tenant_size_with_multiple_branches: debug
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_get_tenant_size_with_multiple_branches[debug-pg14]"
Flaky tests (2)

Postgres 16

  • test_crafted_wal_end[last_wal_record_crossing_segment]: release

Postgres 14

  • test_download_remote_layers_api[local_fs]: debug

Test coverage report is not available

The comment gets automatically updated with the latest test results
badfa15 at 2023-09-15T14:03:58.876Z :recycle:

koivunej added a commit that referenced this pull request Sep 15, 2023
Split off from #5297.

There should be no functional changes here:
- refactor tenant metric "production" like previously timeline, allows
unit testing, though not interesting enough yet to test
- introduce type aliases for tuples
- extra refactoring for `collect`, was initially thinking it was useful
but will do a inline later
- shorter binding names
- support for future allocation reuse quests with IdempotencyKey
- move code out of tokio::select to make it rustfmt-able
- generification, allow later replacement of `&'static str` with enum
- add tests that assert sent event contents exactly
koivunej added a commit that referenced this pull request Sep 15, 2023
We no longer use pageserver deduplication anywhere. Give out a warning
instead.

Split off from #5297.

Cc: #5175 for dedup.
koivunej added a commit that referenced this pull request Sep 15, 2023
Split off from #5297. Depends on #5315.
Cc: #5175 for retry
@koivunej koivunej changed the title fix: consumption metrics on restart ongoing: fix: consumption metrics on restart Sep 15, 2023
koivunej added a commit that referenced this pull request Sep 16, 2023
…5324)

With the addition of the "stateful event verification" the
test_consumption_metrics.py is now too crowded. Split it up for
pageserver and proxy.

Split from #5297.
@koivunej
Copy link
Member Author

This is no longer needed.

@koivunej koivunej closed this Sep 16, 2023
@koivunej koivunej deleted the refactor_consumption_metrics_test branch September 16, 2023 15:06
koivunej added a commit that referenced this pull request Sep 16, 2023
Cleanups in preparation to splitting the consumption_metrics.rs in
#5326.

Split off from #5297.
koivunej added a commit that referenced this pull request Sep 16, 2023
Split off from #5297. Builds upon #5325, should contain only the
splitting. Next up: #5327.
koivunej added a commit that referenced this pull request Sep 18, 2023
Split off from #5297. Builds upon #5326. Handles original review
comments which I did not move to earlier split PRs. Completes test
support for verifying events by notifying of the last batch of events.
Adds cleaning up of tempfiles left because of an unlucky shutdown or
SIGKILL.

Finally closes #5175.

Co-authored-by: Arpad Müller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

consumption_metrics: sending previously sent values on restart consumption_metrics: next steps
3 participants