Skip to content

Commit

Permalink
fix(server): Always apply cache debouncing for project states (#819)
Browse files Browse the repository at this point in the history
Removes the `is_local` override for all project state sources for
multiple reasons:

- Reduces pressure on the Redis source.
- Reduces pressure on the contended project cache actor and the project
  sources.
- Applies equivalent caching across all sources.
  • Loading branch information
jan-auer authored Oct 23, 2020
1 parent 8bd339b commit 043dfe2
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 35 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@

## Unreleased

**Features**:

- Rename upstream retries histogram metric and add upstream requests duration metric. ([#816](https://github.com/getsentry/relay/pull/816))

**Internal**:

- Always apply cache debouncing for project states. This reduces pressure on the Redis and file system cache. ([#819](https://github.com/getsentry/relay/pull/819))

## 20.10.1

**Internal**:
Expand Down
20 changes: 5 additions & 15 deletions relay-server/src/actors/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,6 @@ pub struct Project {
state: Option<Arc<ProjectState>>,
state_channel: Option<Shared<oneshot::Receiver<Arc<ProjectState>>>>,
rate_limits: RateLimits,
is_local: bool,
}

impl Project {
Expand All @@ -421,7 +420,6 @@ impl Project {
state: None,
state_channel: None,
rate_limits: RateLimits::new(),
is_local: false,
}
}

Expand All @@ -441,19 +439,15 @@ impl Project {
.map(|s| s.outdated(&self.config))
.unwrap_or(Outdated::HardOutdated);

let alternative_rv = match (state, outdated, self.is_local) {
// The state is fetched from a local file, don't use own caching logic. Rely on
// `ProjectCache#local_states` for caching.
(_, _, true) => None,

let cached_state = match (state, outdated) {
// There is no project state that can be used, fetch a state and return it.
(None, _, false) | (_, Outdated::HardOutdated, false) => None,
(None, _) | (_, Outdated::HardOutdated) => None,

// The project is semi-outdated, fetch new state but return old one.
(Some(state), Outdated::SoftOutdated, false) => Some(state.clone()),
(Some(state), Outdated::SoftOutdated) => Some(state.clone()),

// The project is not outdated, return early here to jump over fetching logic below.
(Some(state), Outdated::Updated, false) => return Response::ok(state.clone()),
(Some(state), Outdated::Updated) => return Response::ok(state.clone()),
};

let channel = match self.state_channel {
Expand All @@ -469,7 +463,7 @@ impl Project {
}
};

if let Some(rv) = alternative_rv {
if let Some(rv) = cached_state {
return Response::ok(rv);
}

Expand All @@ -492,10 +486,6 @@ impl Project {
.into_actor(self)
.map(move |state_result, slf, _ctx| {
slf.state_channel = None;
slf.is_local = state_result
.as_ref()
.map(|resp| resp.is_local)
.unwrap_or(false);
slf.state = state_result.map(|resp| resp.state).ok();

if let Some(ref state) = slf.state {
Expand Down
25 changes: 7 additions & 18 deletions relay-server/src/actors/project_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,22 +176,11 @@ pub struct FetchProjectState {
#[derive(Debug)]
pub struct ProjectStateResponse {
pub state: Arc<ProjectState>,
pub is_local: bool,
}

impl ProjectStateResponse {
pub fn managed(state: Arc<ProjectState>) -> Self {
ProjectStateResponse {
state,
is_local: false,
}
}

pub fn local(state: Arc<ProjectState>) -> Self {
ProjectStateResponse {
state,
is_local: true,
}
pub fn new(state: Arc<ProjectState>) -> Self {
ProjectStateResponse { state }
}
}

Expand Down Expand Up @@ -239,23 +228,23 @@ impl Handler<FetchProjectState> for ProjectCache {

let future = fetch_local.and_then(move |response| {
if let Some(state) = response {
return Box::new(future::ok(ProjectStateResponse::local(state)))
return Box::new(future::ok(ProjectStateResponse::new(state)))
as ResponseFuture<_, _>;
}

match relay_mode {
RelayMode::Proxy => {
return Box::new(future::ok(ProjectStateResponse::local(Arc::new(
return Box::new(future::ok(ProjectStateResponse::new(Arc::new(
ProjectState::allowed(),
))));
}
RelayMode::Static => {
return Box::new(future::ok(ProjectStateResponse::local(Arc::new(
return Box::new(future::ok(ProjectStateResponse::new(Arc::new(
ProjectState::missing(),
))));
}
RelayMode::Capture => {
return Box::new(future::ok(ProjectStateResponse::local(Arc::new(
return Box::new(future::ok(ProjectStateResponse::new(Arc::new(
ProjectState::allowed(),
))));
}
Expand All @@ -280,7 +269,7 @@ impl Handler<FetchProjectState> for ProjectCache {

let fetch_redis = fetch_redis.and_then(move |response| {
if let Some(state) = response {
return Box::new(future::ok(ProjectStateResponse::local(state)))
return Box::new(future::ok(ProjectStateResponse::new(state)))
as ResponseFuture<_, _>;
}

Expand Down
2 changes: 1 addition & 1 deletion relay-server/src/actors/project_upstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ impl Handler<FetchProjectState> for UpstreamProjectSource {
channel
.receiver()
.map_err(|_| ())
.map(|x| ProjectStateResponse::managed((*x).clone())),
.map(|x| ProjectStateResponse::new((*x).clone())),
)
}
}
5 changes: 4 additions & 1 deletion tests/integration/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@

def test_local_project_config(mini_sentry, relay):
config = mini_sentry.basic_project_config()
relay = relay(mini_sentry, {"cache": {"file_interval": 1}}, wait_healthcheck=False)
relay_config = {
"cache": {"file_interval": 1, "project_expiry": 0, "project_grace_period": 0}
}
relay = relay(mini_sentry, relay_config, wait_healthcheck=False)
relay.config_dir.mkdir("projects").join("42.json").write(
json.dumps(
{
Expand Down

0 comments on commit 043dfe2

Please sign in to comment.