neon icon indicating copy to clipboard operation
neon copied to clipboard

Add support for PS sharding in compute

Open knizhnik opened this issue 1 year ago • 4 comments

refer #5508

replaces #5837

Problem

This PR implements sharding support at compute side. Relations are splinted in stripes and get_page requests are redirected to the particular shard where stripe is located. All other requests (i.e. get relation or database size) are always send to shard 0.

Summary of changes

Support of sharding at compute side include three things:

  1. Make it possible to specify and change in runtime connection to more retain one page server
  2. Send get_page request to the particular shard (determined by hash of page key)
  3. Support multiple servers in prefetch ring requests

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

knizhnik avatar Dec 20 '23 19:12 knizhnik

2286 tests run: 2195 passed, 0 failed, 91 skipped (full report)


Flaky tests (1)

Postgres 14

  • test_statvfs_pressure_usage: debug

Code coverage (full report)

  • functions: 54.4% (10729 of 19706 functions)
  • lines: 81.4% (60917 of 74876 lines)

The comment gets automatically updated with the latest test results
d53f1e8e4819b31b3950f6e81059970e38728236 at 2024-01-25T13:24:06.919Z :recycle:

github-actions[bot] avatar Dec 20 '23 19:12 github-actions[bot]

Two things before merging:

  • Please check in with Heikki: he was a reviewer on the previous PR
  • Please write a proper description, as this will end up in the commit message

jcsp avatar Dec 20 '23 20:12 jcsp

Two things before merging:

  • Please check in with Heikki: he was a reviewer on the previous PR
  • Please write a proper description, as this will end up in the commit message

Done

knizhnik avatar Dec 21 '23 06:12 knizhnik

Please separate the locking changes around reloading postgresql.conf to a separate PR. That'll make both this and the new PR easier to review.

please.

This also changes a lot of elog(...) calls to neon_log(...). Please do that as a separate PR as well, for same reasons.

hlinnaka avatar Jan 09 '24 18:01 hlinnaka

This also changes a lot of elog(...) calls to neon_log(...). Please do that as a separate PR as well, for same reasons.

https://github.com/neondatabase/neon/pull/6313

knizhnik avatar Jan 10 '24 14:01 knizhnik

Please separate the locking changes around reloading postgresql.conf to a separate PR. That'll make both this and the new PR easier to review.

https://github.com/neondatabase/neon/pull/6314

knizhnik avatar Jan 10 '24 15:01 knizhnik

Is this ready for another round of review?

hlinnaka avatar Jan 23 '24 22:01 hlinnaka

Is this ready for another round of review?

Yes, I rebased it with main

knizhnik avatar Jan 24 '24 07:01 knizhnik

This PR overwrites a lot of the changes that were made in #6314, and moves code around, making it again hard to see what the real changes are. I'll spend some time to move things back, to make this easier to review.

hlinnaka avatar Jan 24 '24 11:01 hlinnaka

How to test this PR?

hlinnaka avatar Jan 24 '24 11:01 hlinnaka

How to test this PR?

There's a circular dependency: this PR depends on the changes in the rest of the stack for testing (https://github.com/neondatabase/neon/pull/6380), and they depend on this PR to be able to test sharding. So something has to merge without tests.

jcsp avatar Jan 24 '24 12:01 jcsp

This PR overwrites a lot of the changes that were made in #6314, and moves code around, making it again hard to see what the real changes are. I'll spend some time to move things back, to make this easier to review.

Ok, I added a commit that reverts the overwriting of #6314 changes, making this much smaller. It compiles, but I have done zero testing. @knizhnik can you please try this out and fix whatever I broke?

I'll add some review comments explaining the changes I made

hlinnaka avatar Jan 24 '24 14:01 hlinnaka

There are unresolved conversations which prevents merge and which can not be located and so resolved because of force push

knizhnik avatar Jan 25 '24 08:01 knizhnik

Strange, I can see conversations when I scroll up (sometimes github collapses part of the history and you have to click a divider to expand it): image

jcsp avatar Jan 25 '24 09:01 jcsp