Skip to content

Conversation

@ggevay
Copy link
Contributor

@ggevay ggevay commented Oct 26, 2025

This is for running Nightly's scalability benchmark on some experimental optimizations, which could be follow-ups to #33713. CI's machines might be more powerful than my laptop at high parallelism, and I'd like to test the scalability of peek sequencing.

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

ggevay added 30 commits October 24, 2025 13:22
ggevay added 23 commits October 24, 2025 13:22
I think this superfluous error variant existed because earlier
acquire_read_holds's parameters used to specify the time, but then
the interface was changed to just acquire the earliest possible
read hold, and we forgot to drop this error variant.
…to the storage controller

in order to be able to use this instead of ReadHoldError in the next
commit.
and make use of the fact that acquire_read_holds now returns a common
error.
…rn a more specific error

and make use of this in acquire_read_holds_and_least_valid_write
…eturn a more specific error

and make use of this in acquire_read_holds_and_least_valid_write
We had a bunch of places with a `&mut Catalog` or an `Arc<Catalog>`
where a simple `&Catalog` is enough. `&Catalog` is better
performance-wise than `&mut Catalog`, because creating the latter
(with calling `catalog_mut`) clones the whole catalog if there are
any other Arcs to it. Similarly, `&Catalog` is also better than an
`Arc<Catalog>`, because the latter's existence can cause other
`catalog_mut` calls to make a clone.
so that the next commit needs to work only with SessionClient's
catalog_snapshot, and not with PeekClient's catalog_snapshot
(which is deleted in this commit).
@ggevay ggevay force-pushed the peek-frontend-catalog-snapshot-elimination branch from f4346b4 to 044d037 Compare October 26, 2025 18:07
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.

1 participant