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

pageserver_remote_physical_size metric lags behind #3738

Closed
problame opened this issue Mar 2, 2023 · 1 comment · Fixed by #3741
Closed

pageserver_remote_physical_size metric lags behind #3738

problame opened this issue Mar 2, 2023 · 1 comment · Fixed by #3741
Assignees
@problame

Comments

@problame
Copy link
Contributor

problame commented Mar 2, 2023

While implementing #3729 I noticed that the pageserver_remote_physical_size lags behind.

So, I commented out that part of the test.

But we need to get to the bottom of this.
@problame problame self-assigned this Mar 2, 2023
@problame problame mentioned this issue Mar 2, 2023
Merged
@problame problame mentioned this issue Mar 3, 2023
Merged
problame added a commit that referenced this issue Mar 3, 2023
@problame
fix: compaction: no index upload scheduled if no on-demand downloads
1ec0fd7 
Commit

    0cf7fd0
    Compaction with on-demand download (#3598)

introduced a subtle bug: if we don't have to do on-demand downloads,
we only take one ROUND in fn compact() and exit early.
Thereby, we miss scheduling the index part upload for any layers
created by fn compact_inner().

Before that commit, we didn't have this problem.
So, this patch fixes it.

Since no regression test caught this, I went ahead and extended the
timeline size tests to assert that, if remote storage is configured,
1. pageserver_remote_physical_size matches the other physical sizes
2. file sizes reported by the layer map info endpoint match the other
   physical size metrics

Without the pageserver code fix, the regression test would
fail at the physical size assertion, complaining that
any of the resident physical size != remote physical size metric
50790400.0 != 18399232.0
I figured out what the problem is by comparing the remote storage
and local directories like so, and noticed that the image layer
in the local directory wasn't present on the remote side.
It's size was exactly the difference
    50790400.0 - 18399232.0  =32391168.0

fixes #3738
problame added a commit that referenced this issue Mar 3, 2023
@problame
fix: compaction: no index upload scheduled if no on-demand downloads
9655709 
Commit

    0cf7fd0
    Compaction with on-demand download (#3598)

introduced a subtle bug: if we don't have to do on-demand downloads,
we only take one ROUND in fn compact() and exit early.
Thereby, we miss scheduling the index part upload for any layers
created by fn compact_inner().

Before that commit, we didn't have this problem.
So, this patch fixes it.

Since no regression test caught this, I went ahead and extended the
timeline size tests to assert that, if remote storage is configured,
1. pageserver_remote_physical_size matches the other physical sizes
2. file sizes reported by the layer map info endpoint match the other
   physical size metrics

Without the pageserver code fix, the regression test would
fail at the physical size assertion, complaining that
any of the resident physical size != remote physical size metric
50790400.0 != 18399232.0
I figured out what the problem is by comparing the remote storage
and local directories like so, and noticed that the image layer
in the local directory wasn't present on the remote side.
It's size was exactly the difference
    50790400.0 - 18399232.0  =32391168.0

fixes #3738
problame added a commit that referenced this issue Mar 3, 2023
@problame
fix: compaction: no index upload scheduled if no on-demand downloads
66a5159 
Commit

    0cf7fd0
    Compaction with on-demand download (#3598)

introduced a subtle bug: if we don't have to do on-demand downloads,
we only take one ROUND in fn compact() and exit early.
Thereby, we miss scheduling the index part upload for any layers
created by fn compact_inner().

Before that commit, we didn't have this problem.
So, this patch fixes it.

Since no regression test caught this, I went ahead and extended the
timeline size tests to assert that, if remote storage is configured,
1. pageserver_remote_physical_size matches the other physical sizes
2. file sizes reported by the layer map info endpoint match the other
   physical size metrics

Without the pageserver code fix, the regression test would
fail at the physical size assertion, complaining that
any of the resident physical size != remote physical size metric
50790400.0 != 18399232.0
I figured out what the problem is by comparing the remote storage
and local directories like so, and noticed that the image layer
in the local directory wasn't present on the remote side.
It's size was exactly the difference
    50790400.0 - 18399232.0  =32391168.0

fixes #3738
@koivunej
Copy link
Member

Will be interesting to see if these tests become flaky, since layermap is more consistent than the relaxed counters/gauges which could have whatever value even on amd64.

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

@problame problame

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: compaction: no index upload scheduled if no on-demand downloads neondatabase/neon
2 participants
@problame @koivunej