Skip to content

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Jul 7, 2023

Problem

We were computing margin by lsn range, but this will cause problems for layer maps with large overlapping LSN range. Now we compute x, y margin separately to avoid this issue.

Summary of changes

before:

image

we have a lot of rectangles of negative width, and they disappear in the layer map.

after:

image

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 July 7, 2023 16:02
@skyzh skyzh requested review from fprasx and problame and removed request for a team July 7, 2023 16:02
@skyzh skyzh force-pushed the skyzh/timeline-dir-fix branch from b253d32 to ae38aae Compare July 7, 2023 16:03
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

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


Flaky tests (1)

Postgres 15

  • test_remote_storage_upload_queue_retries[local_fs]: release
The comment gets automatically updated with the latest test results
ae38aae at 2023-07-07T16:21:13.879Z :recycle:

@skyzh skyzh requested a review from bojanserafimov July 10, 2023 12:04
Copy link
Contributor

@bojanserafimov bojanserafimov left a comment

Choose a reason for hiding this comment

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

I'm not very proud of this code 😅 Thanks for the fix

@skyzh skyzh merged commit 5177c1e into main Jul 10, 2023
@skyzh skyzh deleted the skyzh/timeline-dir-fix branch July 10, 2023 13:22
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.

2 participants