control_plane/attachment_service: complete APIs

[jcsp]

Depends on: #6468

Problem

The sharding service will be used as a "virtual pageserver" by the control plane -- so it needs the set of pageserver APIs that the control plane uses, and to present them under identical URLs, including prefix (/v1).

Summary of changes

• Add missing APIs:
  • Tenant deletion
  • Timeline deletion
  • Node list (used in test now, later in tools)
  • /location_config API (for migrating tenants into the sharding service)
• Rework attachment service URLs:
  • /v1 prefix is used for pageserver-compatible APIs
  • /upcall/v1 prefix is used for APIs that are called by the pageserver (re-attach and validate)
  • /debug/v1 prefix is used for endpoints that are for testing
  • /control/v1 prefix is used for new sharding service APIs that do not mimic a pageserver API, such as registering and configuring nodes.
• Add test_sharding_service. The sharding service already had some collateral coverage from its use in general tests, but this is the first dedicated testing for it.

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

[github-actions]

2364 tests run: 2272 passed, 0 failed, 92 skipped (full report)

---

Flaky tests (2)

Postgres 16

• test_crafted_wal_end[last_wal_record_xlog_switch_ends_on_page_boundary]: debug

Postgres 15

• test_secondary_mode_eviction: debug

Code coverage (full report)

• functions: 54.5% (11143 of 20462 functions)
• lines: 81.6% (62829 of 77015 lines)

---

_{The comment gets automatically updated with the latest test results
c666841 at 2024-01-30T18:42:52.175Z :recycle:}

[save-buffer]

Mostly just for my curiosity, did you by any chance consider using sqlite? It seems like an in-process database could save some complexity.

[jcsp]

Mostly just for my curiosity, did you by any chance consider using sqlite? It seems like an in-process database could save some complexity.

Yes -- postgres is used because that's what we'll use in production: I don't want to test against a different database than we run with. It also simplifies life to avoid dealing with two SQL dialects. The impact to test runtime from adding a postgres process is very small, but if we want to eliminate it I'd do it by just adding a non-persistent mode to the attachment service (i.e. use no database at all), since in most tests it is never restarted.

[problame]

I like the new distinction between upcall & control plane calls.

---

Regarding tenant_location_config: would prefer if, isntead of special-casing creation later, it would detect the need to create first, do it if necessary, then enter common code path of update. Rationale: it de-special-cases the creation to some extend. Doesn't have to be this PR though.

Also regarding tenant_location_config: I didn't keep up with all the changes, but, is this the first API that allows configuring a (single-)sharded tenant?

---

I think I need to spend more time with service.rs, but, wanted to flush my pending review comments by this time.

[jcsp]

Also regarding tenant_location_config: I didn't keep up with all the changes, but, is this the first API that allows configuring a (single-)sharded tenant?xz

If you mean on the pageserver API: Yes, although it has allowed it for some time already: because tenant creation is really just an attach, one can create a multi-sharded tenant by issuing a series of /location_config calls to shards.

If you mean in the attachment service API: no, one could already do it with the tenant creation API.

[jcsp]

Regarding tenant_location_config: would prefer if, isntead of special-casing creation later, it would detect the need to create first, do it if necessary, then enter common code path of update. Rationale: it de-special-cases the creation to some extend. Doesn't have to be this PR though.

This code is refactored in #6521 -- the create and update paths are still separate, but in a more obvious way that avoids having to read back and forth in the function.

[problame]

Wrote a lengthy comment on the location config API.

I think the idea of the "virtual pageserver" API is a good approach to offload work off the control plane team, but, the fact that we're sometimes transferring authority over generation numbers bears a too-high-risk is too risky for my taste, as pointed out in the comment.

Proposal: continue with virtual pageserver API approach, but, don't move authority over generation numbers. Instead, have this service call to the generation numbers service, just like the control plane code does.

None of these concerns are directly related to this specific PR, which I'll approve to unblock you.

But, I'm medium-to-strongly opposed to putting it into production until we have found a way to de-risk the generation numbers authority thing.