Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxy: random changes #8602

Merged
merged 4 commits into from
Aug 7, 2024
Merged

proxy: random changes #8602

merged 4 commits into from
Aug 7, 2024

Conversation

conradludgate
Copy link
Contributor

@conradludgate conradludgate commented Aug 5, 2024

Problem

  1. Hard to correlate startup parameters with the endpoint that provided them.
  2. Some configurations are not needed in the ProxyConfig struct.

Summary of changes

Because of some borrow checker fun, I needed to switch to an interior-mutability implementation of our RequestMonitoring context system. Using https://docs.rs/try-lock/latest/try_lock/ as a cheap lock for such a use-case (needed to be thread safe).

Removed the lock of each startup message, instead just logging only the startup params in a successful handshake.

Also removed from values from ProxyConfig and kept as arguments. (needed for local-proxy config)

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

@conradludgate conradludgate requested a review from a team as a code owner August 5, 2024 11:45
Copy link
Contributor

@stradig stradig left a comment

Choose a reason for hiding this comment

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

LGTM

proxy/src/context.rs Show resolved Hide resolved
@conradludgate conradludgate enabled auto-merge (squash) August 6, 2024 12:07
Copy link

github-actions bot commented Aug 6, 2024

2112 tests run: 2043 passed, 0 failed, 69 skipped (full report)


Flaky tests (1)

Postgres 14

Code coverage* (full report)

  • functions: 32.7% (7161 of 21929 functions)
  • lines: 50.5% (57774 of 114354 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
b1930d2 at 2024-08-06T13:48:24.549Z :recycle:

@conradludgate conradludgate merged commit ad0988f into main Aug 7, 2024
63 checks passed
@conradludgate conradludgate deleted the shrink-startup-logs branch August 7, 2024 13:37
jcsp pushed a commit that referenced this pull request Aug 12, 2024
## Problem

1. Hard to correlate startup parameters with the endpoint that provided
them.
2. Some configurations are not needed in the `ProxyConfig` struct.

## Summary of changes

Because of some borrow checker fun, I needed to switch to an
interior-mutability implementation of our `RequestMonitoring` context
system. Using https://docs.rs/try-lock/latest/try_lock/ as a cheap lock
for such a use-case (needed to be thread safe).

Removed the lock of each startup message, instead just logging only the
startup params in a successful handshake.

Also removed from values from `ProxyConfig` and kept as arguments.
(needed for local-proxy config)
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