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

dump more info in layer map #4567

Merged
merged 7 commits into from
Jul 6, 2023
Merged

dump more info in layer map #4567

merged 7 commits into from
Jul 6, 2023

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Jun 26, 2023

Problem

A simple commit extracted from #4539

Summary of changes

This PR adds more info for layer dumps (is_delta, is_incremental, size).

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Do not forget to reformat commit message to not include the above checklist

@skyzh skyzh requested review from a team as code owners June 26, 2023 19:43
@skyzh skyzh requested review from knizhnik and LizardWizzard and removed request for a team June 26, 2023 19:44
Signed-off-by: Alex Chi <[email protected]>
@github-actions
Copy link

github-actions bot commented Jun 26, 2023

1016 tests run: 973 passed, 0 failed, 43 skipped (full report)


Flaky tests (1)

Postgres 15

  • test_remote_timeline_client_calls_started_metric[local_fs]: release
The comment gets automatically updated with the latest test results
f2f327f at 2023-07-06T14:47:33.696Z :recycle:

@LizardWizzard
Copy link
Contributor

the image layer may only contain part of the images within the key range.

Why not to shrink the key range instead? Or incremental layer can contain images from entire key range?

@skyzh
Copy link
Member Author

skyzh commented Jun 27, 2023

Or incremental layer can contain images from entire key range?

Incremental layer contains part of the keys from an entire key range. Previously, we will scan all keys within the range to construct the layer so that we don't need to search more once we hit the image layer. However, it is costly to construct such image layers, because we need to scan every key in the range. Therefore, with incremental image layer, an image layer can only contain some keys within the key range, and we can decide whether to generate an image for a key based on some heuristics.

@skyzh
Copy link
Member Author

skyzh commented Jun 29, 2023

this might cause we miss the delta records == image_lsn, converting to draft.

@skyzh skyzh marked this pull request as draft June 29, 2023 17:00
Signed-off-by: Alex Chi Z <[email protected]>
@skyzh skyzh changed the title support incremental image layer and dump more info dump more info in layer map Jun 29, 2023
@skyzh
Copy link
Member Author

skyzh commented Jun 29, 2023

The true implementation for incremental layer is more complex than I thought and therefore I excluded it from this PR. Now we only have the dump more info thing in this PR and appreciate an easy approval from someone :)

@skyzh skyzh marked this pull request as ready for review June 29, 2023 19:10
Signed-off-by: Alex Chi Z <[email protected]>
@skyzh skyzh requested a review from bojanserafimov July 3, 2023 19:20
@skyzh skyzh enabled auto-merge (squash) July 6, 2023 14:25
@skyzh skyzh force-pushed the skyzh/partial-image-layer branch from 81d00e9 to f2f327f Compare July 6, 2023 14:26
@skyzh skyzh merged commit d340cf3 into main Jul 6, 2023
@skyzh skyzh deleted the skyzh/partial-image-layer branch July 6, 2023 15:21
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.

3 participants