Skip to content

Commit

Permalink
chore: smaller layer changes (#9247)
Browse files Browse the repository at this point in the history
Address minor technical debt in Layer inspired by #9224:

- layer usage as arg same as in spans
- avoid one Weak::upgrade
  • Loading branch information
koivunej authored Oct 3, 2024
1 parent 6a9e2d6 commit dbef1b0
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions pageserver/src/tenant/storage_layer/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,8 @@ impl Layer {
// This case is legal in brief time windows: for example an in-flight getpage request can hold on to a layer object
// which was covered by a concurrent compaction.
tracing::info!(
"Layer {} became visible as a result of access",
self.0.desc.layer_name()
layer=%self,
"became visible as a result of access",
);
}
}
Expand Down Expand Up @@ -688,7 +688,9 @@ impl Drop for LayerInner {
// and we could be delaying shutdown for nothing.
}

if let Some(timeline) = self.timeline.upgrade() {
let timeline = self.timeline.upgrade();

if let Some(timeline) = timeline.as_ref() {
// Only need to decrement metrics if the timeline still exists: otherwise
// it will have already de-registered these metrics via TimelineMetrics::shutdown
if self.desc.is_delta() {
Expand Down Expand Up @@ -719,7 +721,6 @@ impl Drop for LayerInner {
let path = std::mem::take(&mut self.path);
let file_name = self.layer_desc().layer_name();
let file_size = self.layer_desc().file_size;
let timeline = self.timeline.clone();
let meta = self.metadata();
let status = self.status.take();

Expand All @@ -729,7 +730,7 @@ impl Drop for LayerInner {
// carry this until we are finished for [`Layer::wait_drop`] support
let _status = status;

let Some(timeline) = timeline.upgrade() else {
let Some(timeline) = timeline else {
// no need to nag that timeline is gone: under normal situation on
// task_mgr::remove_tenant_from_memory the timeline is gone before we get dropped.
LAYER_IMPL_METRICS.inc_deletes_failed(DeleteFailed::TimelineGone);
Expand Down

1 comment on commit dbef1b0

@github-actions
Copy link

Choose a reason for hiding this comment

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

5146 tests run: 4949 passed, 1 failed, 196 skipped (full report)


Failures on Postgres 16

  • test_branch_creation_many[github-actions-selfhosted-random-500]: release-x86-64
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_branch_creation_many[release-pg16-github-actions-selfhosted-random-500]"
Flaky tests (4)

Postgres 17

Postgres 16

Postgres 15

Postgres 14

Code coverage* (full report)

  • functions: 31.4% (7489 of 23883 functions)
  • lines: 49.6% (60129 of 121348 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
dbef1b0 at 2024-10-03T10:59:23.744Z :recycle:

Please sign in to comment.