Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

Commit

Permalink
feat: add the fallback protocol (#464)
Browse files Browse the repository at this point in the history
to render Cache-Control headers in most cases

and fix omissions of stack traces to sentry events

Closes #305
Closes #376
  • Loading branch information
pjenvey authored Oct 14, 2022
1 parent 2a7ece0 commit 56ee834
Show file tree
Hide file tree
Showing 9 changed files with 327 additions and 85 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ blake3 = "1"
bytes = "1"
cadence = "0.29"
chrono = "0.4"
crossbeam-channel = "0.5.4"
docopt = "1.1"
cloud-storage = { git = "https://github.com/mozilla-services/cloud-storage-rs", branch = "release/0.11.1-client-builder-and-params" }
config = "0.13"
Expand Down
2 changes: 1 addition & 1 deletion src/adm/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl AdmFilter {
// TODO: if not error.is_reportable, just add to metrics.
let mut merged_tags = error.tags.clone();
merged_tags.extend(tags.clone());
l_sentry::report(sentry::event_from_error(error), &merged_tags);
l_sentry::report(error, &merged_tags);
}

/// check to see if the bucket has been modified since the last time we updated.
Expand Down
4 changes: 2 additions & 2 deletions src/adm/tiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
server::ServerState,
settings::Settings,
tags::Tags,
web::middleware::sentry::report,
web::middleware::sentry as l_sentry,
web::DeviceInfo,
};

Expand Down Expand Up @@ -274,7 +274,7 @@ pub async fn get_tiles(
}
Err(e) => {
// quietly report the error, and drop the tile.
report(sentry::event_from_error(&e), tags);
l_sentry::report(&e, tags);
continue;
}
}
Expand Down
79 changes: 72 additions & 7 deletions src/server/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use std::{
time::{Duration, SystemTime},
};

use actix_web::rt;
use actix_web::{
http::header::{CacheControl, CacheDirective, TryIntoHeaderPair},
rt, HttpResponse,
};
use cadence::StatsdClient;
use dashmap::DashMap;

Expand Down Expand Up @@ -75,12 +78,17 @@ impl TilesCache {
audience_key: &'a AudienceKey,
expired: bool,
) -> WriteHandle<'a, impl FnOnce(()) + '_> {
let mut fallback_tiles = None;

if expired {
// The cache entry's expired and we're about to refresh it
trace!("prepare_write: Fresh now expired, Refreshing");
self.inner
.alter(audience_key, |_, tiles_state| match tiles_state {
TilesState::Fresh { tiles } if tiles.expired() => {
// In case an error occurs while doing the write work
// we'll render the current value as a fallback
fallback_tiles = Some(tiles.clone());
TilesState::Refreshing { tiles }
}
_ => tiles_state,
Expand All @@ -95,8 +103,8 @@ impl TilesCache {
let guard = scopeguard::guard((), move |_| {
trace!("prepare_write (ScopeGuard cleanup): Resetting state");
if expired {
// Back to Fresh (though the tiles are expired): so a later request
// will retry refreshing again
// Back to Fresh (though the tiles are expired): so a later
// request will retry refreshing again
self.inner
.alter(audience_key, |_, tiles_state| match tiles_state {
TilesState::Refreshing { tiles } => TilesState::Fresh { tiles },
Expand All @@ -113,6 +121,7 @@ impl TilesCache {
cache: self,
audience_key,
guard,
fallback_tiles,
}
}
}
Expand All @@ -129,6 +138,7 @@ where
cache: &'a TilesCache,
audience_key: &'a AudienceKey,
guard: scopeguard::ScopeGuard<(), F>,
pub fallback_tiles: Option<Tiles>,
}

impl<F> WriteHandle<'_, F>
Expand Down Expand Up @@ -169,12 +179,22 @@ impl TilesState {
#[derive(Clone, Debug)]
pub struct Tiles {
pub content: TilesContent,
/// When this is in need of a refresh (the `Cache-Control` `max-age`)
expiry: SystemTime,
/// After expiry we'll continue serving the stale version of these Tiles
/// until they're successfully refreshed (acting as a fallback during
/// upstream service outages). `fallback_expiry` is when we stop serving
/// this stale Tiles completely
fallback_expiry: SystemTime,
}

impl Tiles {
pub fn new(tile_response: TileResponse, ttl: u32) -> Result<Self, HandlerError> {
let empty = Self::empty(ttl);
pub fn new(
tile_response: TileResponse,
ttl: Duration,
fallback_ttl: Duration,
) -> Result<Self, HandlerError> {
let empty = Self::empty(ttl, fallback_ttl);
if tile_response.tiles.is_empty() {
return Ok(empty);
}
Expand All @@ -186,16 +206,61 @@ impl Tiles {
})
}

pub fn empty(ttl: u32) -> Self {
pub fn empty(ttl: Duration, fallback_ttl: Duration) -> Self {
Self {
content: TilesContent::Empty,
expiry: SystemTime::now() + Duration::from_secs(ttl as u64),
expiry: SystemTime::now() + ttl,
fallback_expiry: SystemTime::now() + fallback_ttl,
}
}

pub fn expired(&self) -> bool {
self.expiry <= SystemTime::now()
}

pub fn fallback_expired(&self) -> bool {
self.fallback_expiry <= SystemTime::now()
}

pub fn to_response(&self, cache_control_header: bool) -> HttpResponse {
match &self.content {
TilesContent::Json(json) => {
let mut builder = HttpResponse::Ok();
if cache_control_header {
builder.insert_header(self.cache_control_header());
}
builder
.content_type("application/json")
.body(json.to_owned())
}
TilesContent::Empty => {
let mut builder = HttpResponse::NoContent();
if cache_control_header {
builder.insert_header(self.cache_control_header());
}
builder.finish()
}
}
}

/// Return the Tiles' `Cache-Control` header
fn cache_control_header(&self) -> impl TryIntoHeaderPair {
let max_age = (self.expiry.duration_since(SystemTime::now()))
.unwrap_or_default()
.as_secs();
let stale_if_error = (self.fallback_expiry.duration_since(SystemTime::now()))
.unwrap_or_default()
.as_secs();
let header_value = CacheControl(vec![
CacheDirective::Private,
CacheDirective::MaxAge(max_age as u32),
CacheDirective::Extension(
"stale-if-error".to_owned(),
Some(stale_if_error.to_string()),
),
]);
("Cache-Control", header_value)
}
}

#[derive(Clone, Debug)]
Expand Down
38 changes: 35 additions & 3 deletions src/settings.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Application settings objects and initialization
use std::convert::TryFrom;
use std::path::PathBuf;
use std::{convert::TryFrom, path::PathBuf, time::Duration};

use actix_web::{dev::ServiceRequest, web::Data, HttpRequest};
use config::{Config, ConfigError, Environment, File};
use rand::{thread_rng, Rng};
use serde::Deserialize;

use crate::adm::AdmFilterSettings;
Expand Down Expand Up @@ -66,6 +66,8 @@ pub struct Settings {
pub actix_keep_alive: Option<u64>,
/// Expire tiles after this many seconds (15 * 60s)
pub tiles_ttl: u32,
/// Fallback expiry for tiles after this many seconds (3 * 60 * 60s)
pub tiles_fallback_ttl: u32,
/// path to MaxMind location database
pub maxminddb_loc: Option<PathBuf>,
/// A JSON formatted string of [StorageSettings] related to
Expand Down Expand Up @@ -95,6 +97,8 @@ pub struct Settings {
/// status code or 204s when disabled. See
/// https://github.com/mozilla-services/contile/issues/284
pub excluded_countries_200: bool,
/// Whether Tiles responses may include a `Cache-Control` header
pub cache_control_header: bool,

// TODO: break these out into a PartnerSettings?
/// Adm partner ID (default: "demofeed")
Expand Down Expand Up @@ -127,7 +131,7 @@ pub struct Settings {
pub adm_ignore_advertisers: Option<String>,
/// a JSON list of advertisers to allow for versions of firefox less than 91.
pub adm_has_legacy_image: Option<String>,
/// Percentage of overall time for fetch "jitter".
/// Percentage of overall time for fetch "jitter" (applied to `tiles_ttl` and tiles_fallback_ttl`)
pub jitter: u8,
}

Expand All @@ -143,7 +147,10 @@ impl Default for Settings {
statsd_host: None,
statsd_port: 8125,
actix_keep_alive: None,
/// 15 minutes
tiles_ttl: 15 * 60,
/// 3 hours
tiles_fallback_ttl: 3 * 60 * 60,
maxminddb_loc: None,
storage: "".to_owned(),
test_mode: TestModes::NoTest,
Expand All @@ -157,6 +164,7 @@ impl Default for Settings {
connect_timeout: 2,
request_timeout: 5,
excluded_countries_200: true,
cache_control_header: true,
// ADM specific settings
adm_endpoint_url: "".to_owned(),
adm_partner_id: None,
Expand Down Expand Up @@ -253,6 +261,30 @@ impl Settings {
pub fn banner(&self) -> String {
format!("http://{}:{}", self.host, self.port)
}

pub fn tiles_ttl_with_jitter(&self) -> Duration {
Duration::from_secs(self.add_jitter(self.tiles_ttl) as u64)
}

pub fn tiles_fallback_ttl_with_jitter(&self) -> Duration {
Duration::from_secs(self.add_jitter(self.tiles_fallback_ttl) as u64)
}

/// Calculate the ttl from the settings by taking the tiles_ttl and
/// calculating a jitter that is no more than 50% of the total TTL. It is
/// recommended that "jitter" be 10%.
fn add_jitter(&self, value: u32) -> u32 {
let mut rng = thread_rng();
let ftl = value as f32;
let offset = ftl * (std::cmp::min(self.jitter, 50) as f32 * 0.01);
if offset == 0.0 {
// Don't panic gen_range with an empty range (a tiles_ttl or jitter
// of 0 was specified)
return 0;
}
let jit = rng.gen_range(0.0 - offset..offset);
(ftl + jit) as u32
}
}

impl<'a> From<&'a HttpRequest> for &'a Settings {
Expand Down
Loading

0 comments on commit 56ee834

Please sign in to comment.