Skip to content

Conversation

@jshearer
Copy link
Contributor

@jshearer jshearer commented Apr 28, 2025

Description:

  • Adds a timeout to Metadata API calls, as we found out that these were hanging indefinitely, and consumers were just opening more and more connections as a result. I'd like to have a more global timeout, but that's complicated by the fact that some requests are potentially long-running, such as Fetch with its max_wait_ms parameter. So I went with a more targeted timeout to start with.
  • Skip calls to dekaf::topology::Collection::fetch_spec() if they're not needed -- this reduces the query load on supabase
  • Switch from partition selectors using estuary.dev/collection when there is no binding partition selector to name:prefix. Combined with assemble: Switch journal_selector over to using name:prefix instead of estuary.dev/collection #2108 to include name:prefix in partition selectors themselves, this will cause all calls to fetch_partitions to use name:prefix.
    Note: AFAIK, the only case where we won't have a binding partition selector is when a connection is using the old user-token auth which we're getting rid of ASAP. So the change in this PR is only applicable to sessions using user-token auth, of which there are almost none at this point.

This change is Reviewable

Metadata requests appear to be sometimes hanging forever. We have an idle session timeout, but if a Session is actively handling a request it won't get killed.
@jshearer jshearer marked this pull request as ready for review April 28, 2025 18:17
@jshearer jshearer requested a review from jgraettinger April 28, 2025 18:17
@jshearer jshearer requested a review from a team April 30, 2025 19:23
Copy link
Member

@psFried psFried left a comment

Choose a reason for hiding this comment

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

LGTM

@jshearer jshearer merged commit fb9698c into master Apr 30, 2025
3 checks passed
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