Skip to content

fix: overhaul reports#804

Merged
Theodus merged 4 commits intomainfrom
theodus/reports
Jun 5, 2024
Merged

fix: overhaul reports#804
Theodus merged 4 commits intomainfrom
theodus/reports

Conversation

@Theodus
Copy link
Member

@Theodus Theodus commented Jun 5, 2024

The primary goal of this PR is to increase the amount of concurrent client requests the gateway can handle before p99 latencies start to go up above the latency of indexers responding to the client requests. This happens to align with cleaning up some of the gnarliest tech debt in the gateway that we've accumulated for backwards compatibility for data science. On my local testbench, these changes more than double the amount of concurrent client requests the gateway can handle.

The biggest changes are:

  • use a reporting actor to limit kafka message sends to at most one task at any given time
  • overhaul the client_query module to treat logging, metrics, feedback, kafka, etc. more clearly & deliberately

External impact:

  • The old Client query result & Indexer attempt logs we kept around for internal data science pipelines are gone. Identical field names & values have been maintained in the corresponding kafka outputs. Though some unused fields like status_code and budget have been removed entirely.

@Theodus Theodus requested a review from LNSD June 5, 2024 01:27
Copy link
Contributor

@LNSD LNSD left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@LNSD
Copy link
Contributor

LNSD commented Jun 5, 2024

I agree with the changes made to the client_query.rs file, but we could have waited to make the structural ones after merging the new network resolution module. I get that optimizing how we report the information is critical for the performance. But next time, let's try to keep changes scoped so we don't rework things over and over unnecessarily 😞

@Theodus Theodus merged commit 21833a4 into main Jun 5, 2024
@Theodus Theodus deleted the theodus/reports branch June 5, 2024 14:38
@Theodus Theodus mentioned this pull request Jun 6, 2024
1 task
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