From 83053e0dc2c9a32ef00f40b07826631460fa719c Mon Sep 17 00:00:00 2001 From: Salim Alam Date: Fri, 2 Nov 2018 17:45:22 -0700 Subject: [PATCH 1/3] Perf fixes and caching Signed-off-by: Salim Alam --- .../builder-api/src/server/authorize.rs | 73 +++++---- .../src/server/framework/middleware.rs | 17 +- .../src/server/resources/channels.rs | 151 ++++++++++-------- .../builder-api/src/server/resources/jobs.rs | 20 ++- .../src/server/resources/origins.rs | 62 ++++--- .../builder-api/src/server/resources/pkgs.rs | 53 +++--- .../src/server/resources/profile.rs | 10 +- .../src/server/resources/projects.rs | 4 +- .../builder-api/src/server/resources/user.rs | 4 +- .../src/server/services/memcache.rs | 29 ++++ 10 files changed, 237 insertions(+), 186 deletions(-) diff --git a/components/builder-api/src/server/authorize.rs b/components/builder-api/src/server/authorize.rs index 22495a24e2..3142d40943 100644 --- a/components/builder-api/src/server/authorize.rs +++ b/components/builder-api/src/server/authorize.rs @@ -15,64 +15,59 @@ use actix_web::HttpRequest; use hab_net::privilege::FeatureFlags; -use bldr_core::access_token::{BUILDER_ACCOUNT_ID, BUILDER_ACCOUNT_NAME}; - use protocol::originsrv; -use db::models::account::*; use db::models::origin::*; use server::error::{Error, Result}; use server::AppState; -pub fn authorize_session(req: &HttpRequest, origin_opt: Option<&str>) -> Result { - let account_id = { +pub fn authorize_session( + req: &HttpRequest, + origin_opt: Option<&str>, +) -> Result { + let session = { let extensions = req.extensions(); match extensions.get::() { Some(session) => { let flags = FeatureFlags::from_bits(session.get_flags()).unwrap(); // unwrap Ok if flags.contains(FeatureFlags::BUILD_WORKER) { - return Ok(session.get_id()); + return Ok(session.clone()); } - session.get_id() + session.clone() } None => return Err(Error::Authentication), } }; - let conn = req.state().db.get_conn().map_err(Error::DbError)?; - if let Some(origin) = origin_opt { - match Origin::check_membership(origin, account_id, &*conn).map_err(Error::DieselError) { - Ok(is_member) if is_member => (), - _ => return Err(Error::Authorization), - } - } + let mut memcache = req.state().memcache.borrow_mut(); - Ok(account_id) -} - -// TODO - Merge into authorize_session when we are able to cache the name -pub fn get_session_user_name(req: &HttpRequest, account_id: u64) -> String { - if account_id == BUILDER_ACCOUNT_ID { - return BUILDER_ACCOUNT_NAME.to_string(); - } - - let conn = match req.state().db.get_conn() { - Ok(conn) => conn, - Err(err) => { - warn!("Failed to get account, id={}, err={:?}", account_id, err); - return "".to_string(); + match memcache.get_origin_member(origin, session.get_id()) { + Some(val) => { + if val { + return Ok(session); + } else { + return Err(Error::Authorization); + } + } + None => (), } - }; - match Account::get_by_id(account_id, &*conn) { - Ok(account) => account.name.to_string(), - Err(err) => { - warn!("Failed to get account, id={}, err={:?}", account_id, err); - "".to_string() + match check_origin_member(req, origin, session.get_id()) { + Ok(is_member) => { + memcache.set_origin_member(origin, session.get_id(), is_member); + + match is_member { + true => (), + false => return Err(Error::Authorization), + } + } + _ => return Err(Error::Authorization), } } + + Ok(session) } pub fn check_origin_owner( @@ -87,3 +82,13 @@ pub fn check_origin_owner( Err(err) => Err(err), } } + +pub fn check_origin_member( + req: &HttpRequest, + origin: &str, + account_id: u64, +) -> Result { + let conn = req.state().db.get_conn().map_err(Error::DbError)?; + + Origin::check_membership(origin, account_id, &*conn).map_err(Error::DieselError) +} diff --git a/components/builder-api/src/server/framework/middleware.rs b/components/builder-api/src/server/framework/middleware.rs index b980da4a9a..479c323067 100644 --- a/components/builder-api/src/server/framework/middleware.rs +++ b/components/builder-api/src/server/framework/middleware.rs @@ -21,6 +21,7 @@ use base64; use protobuf; use bldr_core; +use bldr_core::access_token::{BUILDER_ACCOUNT_ID, BUILDER_ACCOUNT_NAME}; use bldr_core::metrics::CounterMetric; use hab_net::{ErrCode, NetError}; use oauth_client::types::OAuth2User; @@ -32,7 +33,6 @@ use db::models::account::*; use hab_net::privilege::FeatureFlags; use server::error; -use server::resources::profile::do_get_access_tokens; use server::services::metrics::Counter; use server::AppState; @@ -107,20 +107,23 @@ fn authenticate(req: &HttpRequest, token: &str) -> error::Result { assert!(access_tokens.len() <= 1); // Can only have max of 1 for now match access_tokens.first() { @@ -133,6 +136,12 @@ fn authenticate(req: &HttpRequest, token: &str) -> error::Result, Query)) -> HttpResponse { let origin = Path::<(String)>::extract(&req).unwrap().into_inner(); - let conn = match req.state().db.get_conn() { + let conn = match req.state().db.get_conn().map_err(Error::DbError) { Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), + Err(err) => return err.into(), }; match Channel::list( @@ -113,8 +111,9 @@ fn get_channels((req, sandbox): (HttpRequest, Query)) -> origin: origin, include_sandbox_channels: sandbox.is_set, }, - conn.deref(), - ) { + &*conn, + ).map_err(Error::DieselError) + { Ok(list) => { // TED: This is to maintain backwards API compat while killing some proto definitions // currently the output looks like [{"name": "foo"}] when it probably should be ["foo"] @@ -131,7 +130,7 @@ fn get_channels((req, sandbox): (HttpRequest, Query)) -> .header(http::header::CACHE_CONTROL, headers::NO_CACHE) .json(ident_list) } - Err(_err) => HttpResponse::InternalServerError().into(), + Err(err) => err.into(), } } @@ -141,22 +140,22 @@ fn create_channel(req: HttpRequest) -> HttpResponse { .into_inner(); let session_id = match authorize_session(&req, Some(&origin)) { - Ok(session_id) => session_id as i64, + Ok(session) => session.get_id(), Err(_) => return HttpResponse::new(StatusCode::UNAUTHORIZED), }; - let conn = match req.state().db.get_conn() { + let conn = match req.state().db.get_conn().map_err(Error::DbError) { Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), + Err(err) => return err.into(), }; match Channel::create( CreateChannel { channel: channel, origin: origin, - owner_id: session_id, + owner_id: session_id as i64, }, - conn.deref(), + &*conn, ) { Ok(channel) => HttpResponse::Created().json(channel), Err(DatabaseError(DatabaseErrorKind::UniqueViolation, _)) => { @@ -179,25 +178,26 @@ fn delete_channel(req: HttpRequest) -> HttpResponse { return HttpResponse::new(StatusCode::FORBIDDEN); } - let conn = match req.state().db.get_conn() { - Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), - }; - req.state() .memcache .borrow_mut() .clear_cache_for_channel(&origin, &channel); + let conn = match req.state().db.get_conn().map_err(Error::DbError) { + Ok(conn_ref) => conn_ref, + Err(err) => return err.into(), + }; + match Channel::delete( DeleteChannel { origin: origin, channel: channel, }, - conn.deref(), - ) { + &*conn, + ).map_err(Error::DieselError) + { Ok(_) => HttpResponse::new(StatusCode::OK), - Err(_) => HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), + Err(err) => err.into(), } } @@ -207,16 +207,11 @@ fn promote_package(req: HttpRequest) -> HttpResponse { .unwrap() .into_inner(); - let session_id = match authorize_session(&req, Some(&origin)) { + let session = match authorize_session(&req, Some(&origin)) { Ok(session) => session, Err(_) => return HttpResponse::new(StatusCode::UNAUTHORIZED), }; - let conn = match req.state().db.get_conn() { - Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), - }; - let ident = PackageIdent::new( origin.clone(), pkg.clone(), @@ -224,6 +219,11 @@ fn promote_package(req: HttpRequest) -> HttpResponse { Some(release.clone()), ); + let conn = match req.state().db.get_conn().map_err(Error::DbError) { + Ok(conn_ref) => conn_ref, + Err(err) => return err.into(), + }; + match OriginChannelPackage::promote( OriginChannelPromote { ident: BuilderPackageIdent(ident.clone()), @@ -231,15 +231,17 @@ fn promote_package(req: HttpRequest) -> HttpResponse { channel: channel.clone(), }, &*conn, - ) { + ).map_err(Error::DieselError) + { Ok(_) => match audit_package_rank_change( &req, &*conn, ident, - channel, + &channel, PackageChannelOperation::Promote, - origin, - session_id, + &origin, + session.get_id(), + session.get_name(), ) { Ok(_) => HttpResponse::new(StatusCode::OK), Err(err) => { @@ -247,7 +249,7 @@ fn promote_package(req: HttpRequest) -> HttpResponse { HttpResponse::new(StatusCode::OK) } }, - Err(_) => HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), + Err(err) => err.into(), } } @@ -261,9 +263,9 @@ fn demote_package(req: HttpRequest) -> HttpResponse { return HttpResponse::new(StatusCode::FORBIDDEN); } - let conn = match req.state().db.get_conn() { - Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), + let session = match authorize_session(&req, Some(&origin)) { + Ok(session) => session, + Err(_) => return HttpResponse::new(StatusCode::UNAUTHORIZED), }; let ident = PackageIdent::new( @@ -272,10 +274,12 @@ fn demote_package(req: HttpRequest) -> HttpResponse { Some(version.clone()), Some(release.clone()), ); - let session_id = match authorize_session(&req, Some(&origin)) { - Ok(session_id) => session_id, - Err(_) => return HttpResponse::new(StatusCode::UNAUTHORIZED), + + let conn = match req.state().db.get_conn().map_err(Error::DbError) { + Ok(conn_ref) => conn_ref, + Err(err) => return err.into(), }; + match OriginChannelPackage::demote( OriginChannelDemote { ident: BuilderPackageIdent(ident.clone()), @@ -283,16 +287,18 @@ fn demote_package(req: HttpRequest) -> HttpResponse { channel: channel.clone(), }, &*conn, - ) { + ).map_err(Error::DieselError) + { Ok(_) => { match audit_package_rank_change( &req, &conn, ident.clone(), - channel, + &channel, PackageChannelOperation::Demote, - origin, - session_id, + &origin, + session.get_id(), + session.get_name(), ) { Ok(_) => {} Err(err) => warn!("Failed to save rank change to audit log: {}", err), @@ -303,7 +309,7 @@ fn demote_package(req: HttpRequest) -> HttpResponse { .clear_cache_for_package(ident.clone().into()); HttpResponse::new(StatusCode::OK) } - Err(_) => HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR), + Err(err) => err.into(), } } @@ -419,20 +425,21 @@ fn audit_package_rank_change( req: &HttpRequest, conn: &PgConnection, ident: PackageIdent, - channel: String, + channel: &str, operation: PackageChannelOperation, - origin: String, + origin: &str, session_id: u64, + user_name: &str, ) -> Result<()> { match PackageChannelAudit::audit( PackageChannelAudit { ident: BuilderPackageIdent(ident), - channel: &channel, + channel: channel, operation: operation, trigger: helpers::trigger_from_request_model(req), requester_id: session_id as i64, - requester_name: &get_session_user_name(req, session_id), - origin: &origin, + requester_name: &user_name, + origin: origin, }, &*conn, ) { @@ -448,14 +455,13 @@ fn do_get_channel_packages( channel: String, ) -> Result<(Vec, i64)> { let opt_session_id = match authorize_session(&req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; + let (page, per_page) = helpers::extract_pagination_in_pages(pagination); let conn = req.state().db.get_conn().map_err(Error::DbError)?; - let (page, per_page) = helpers::extract_pagination_in_pages(pagination); - Channel::list_packages( ListChannelPackages { ident: BuilderPackageIdent(ident.clone().into()), @@ -480,19 +486,13 @@ fn do_get_channel_package( channel: String, ) -> Result { let opt_session_id = match authorize_session(req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; Counter::GetChannelPackage.increment(); - let mut memcache = req.state().memcache.borrow_mut(); let req_ident = ident.clone(); - let conn = match req.state().db.get_conn() { - Ok(conn_ref) => conn_ref, - Err(e) => return Err(e.into()), - }; - // TODO: Deprecate target from headers let target = match qtarget.target.clone() { Some(t) => { @@ -502,14 +502,25 @@ fn do_get_channel_package( None => helpers::target_from_headers(req), }; - match memcache.get_package(req_ident.clone().into(), &channel, &target) { - Some(pkg_json) => { - trace!("Cache Hit!"); - return Ok(pkg_json); - } - None => { - trace!("Cache Miss!"); - } + // Scope this memcache usage so the reference goes out of + // scope before the visibility_for_optional_session call + // below + { + let mut memcache = req.state().memcache.borrow_mut(); + match memcache.get_package(req_ident.clone().into(), &channel, &target) { + Some(pkg_json) => { + trace!("Cache Hit!"); + return Ok(pkg_json); + } + None => { + trace!("Cache Miss!"); + } + }; + } + + let conn = match req.state().db.get_conn() { + Ok(conn_ref) => conn_ref, + Err(e) => return Err(e.into()), }; let pkg = match Channel::get_latest_package( @@ -541,7 +552,11 @@ fn do_get_channel_package( pkg_json["is_a_service"] = json!(is_a_service(&pkg.into())); let json_body = serde_json::to_string(&pkg_json).unwrap(); - memcache.set_package(req_ident.clone(), &json_body, &channel, &target); + + { + let mut memcache = req.state().memcache.borrow_mut(); + memcache.set_package(req_ident.clone(), &json_body, &channel, &target); + } Ok(json_body) } @@ -551,7 +566,7 @@ pub fn channels_for_package_ident( package: &BuilderPackageIdent, ) -> Result>> { let opt_session_id = match authorize_session(req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; diff --git a/components/builder-api/src/server/resources/jobs.rs b/components/builder-api/src/server/resources/jobs.rs index 2c1bd3c72d..6f0d4a41d9 100644 --- a/components/builder-api/src/server/resources/jobs.rs +++ b/components/builder-api/src/server/resources/jobs.rs @@ -32,7 +32,7 @@ use db::models::package::*; use db::models::projects::*; use diesel::result::Error::NotFound; -use server::authorize::{authorize_session, get_session_user_name}; +use server::authorize::authorize_session; use server::error::{Error, Result}; use server::framework::headers; use server::framework::middleware::route_message; @@ -210,7 +210,7 @@ fn do_group_promotion_or_demotion( origin: &str, promote: bool, ) -> Result> { - let account_id = authorize_session(req, Some(&origin))?; + let session = authorize_session(req, Some(&origin))?; let conn = req.state().db.get_conn().map_err(Error::DbError)?; @@ -228,7 +228,7 @@ fn do_group_promotion_or_demotion( CreateChannel { channel: channel.to_string(), origin: origin.to_string(), - owner_id: account_id as i64, + owner_id: session.get_id() as i64, }, &*conn, )? @@ -371,8 +371,7 @@ fn promote_or_demote_job_group( PackageChannelOperation::Demote }; - let session_id = authorize_session(req, None).unwrap(); // Unwrap ok - let session_name = get_session_user_name(req, session_id); + let session = authorize_session(req, None).unwrap(); // Unwrap ok PackageGroupChannelAudit::audit( PackageGroupChannelAudit { @@ -381,8 +380,8 @@ fn promote_or_demote_job_group( pkg_ids: package_ids, operation: pco, trigger: trigger.clone(), - requester_id: session_id as i64, - requester_name: &session_name, + requester_id: session.get_id() as i64, + requester_name: session.get_name(), group_id: group_id as i64, }, &*conn, @@ -475,14 +474,13 @@ fn do_cancel_job_group(req: &HttpRequest, group_id: u64) -> Result = group.get_project_name().split("/").collect(); assert!(name_split.len() == 2); - let session_id = authorize_session(req, Some(&name_split[0]))?; - let session_name = get_session_user_name(req, session_id); + let session = authorize_session(req, Some(&name_split[0]))?; let mut jgc = jobsrv::JobGroupCancel::new(); jgc.set_group_id(group_id); jgc.set_trigger(helpers::trigger_from_request(req)); - jgc.set_requester_id(session_id); - jgc.set_requester_name(session_name); + jgc.set_requester_id(session.get_id()); + jgc.set_requester_name(session.get_name().to_string()); route_message::(req, &jgc) } diff --git a/components/builder-api/src/server/resources/origins.rs b/components/builder-api/src/server/resources/origins.rs index 6677675029..5ade37e0fe 100644 --- a/components/builder-api/src/server/resources/origins.rs +++ b/components/builder-api/src/server/resources/origins.rs @@ -46,7 +46,7 @@ use db::models::origin::*; use db::models::package::{BuilderPackageIdent, ListPackages, Package, PackageVisibility}; use db::models::secrets::*; -use server::authorize::{authorize_session, check_origin_owner, get_session_user_name}; +use server::authorize::{authorize_session, check_origin_owner}; use server::error::{Error, Result}; use server::framework::headers; use server::helpers::{self, Pagination}; @@ -182,9 +182,9 @@ impl Origins { fn get_origin(req: HttpRequest) -> HttpResponse { let origin_name = Path::::extract(&req).unwrap().into_inner(); // Unwrap Ok - let conn = match req.state().db.get_conn() { + let conn = match req.state().db.get_conn().map_err(Error::DbError) { Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::InternalServerError().into(), + Err(err) => return err.into(), }; match Origin::get(&origin_name, &*conn) { @@ -192,20 +192,18 @@ fn get_origin(req: HttpRequest) -> HttpResponse { .header(http::header::CACHE_CONTROL, headers::NO_CACHE) .json(origin), Err(NotFound) => HttpResponse::NotFound().into(), - Err(_) => HttpResponse::InternalServerError().into(), + Err(err) => Error::DieselError(err).into(), } } fn create_origin( (req, body): (HttpRequest, Json), ) -> HttpResponse { - let account_id = match authorize_session(&req, None) { - Ok(id) => id, + let session = match authorize_session(&req, None) { + Ok(session) => session, Err(err) => return err.into(), }; - let account_name = get_session_user_name(&req, account_id); - let dpv = match body.clone().default_package_visibility { Some(viz) => viz, None => PackageVisibility::Public, @@ -215,21 +213,21 @@ fn create_origin( return HttpResponse::ExpectationFailed().into(); } - let conn = match req.state().db.get_conn() { + let conn = match req.state().db.get_conn().map_err(Error::DbError) { Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::InternalServerError().into(), + Err(err) => return err.into(), }; let new_origin = NewOrigin { name: &body.0.name, - owner_id: account_id as i64, - owner_name: &account_name, + owner_id: session.get_id() as i64, + owner_name: session.get_name(), default_package_visibility: &dpv, }; - match Origin::create(&new_origin, &*conn) { + match Origin::create(&new_origin, &*conn).map_err(Error::DieselError) { Ok(origin) => HttpResponse::Created().json(origin), - Err(_e) => HttpResponse::InternalServerError().into(), + Err(err) => err.into(), } } @@ -242,9 +240,9 @@ fn update_origin( return err.into(); } - let conn = match req.state().db.get_conn() { + let conn = match req.state().db.get_conn().map_err(Error::DbError) { Ok(conn_ref) => conn_ref, - Err(_) => return HttpResponse::InternalServerError().into(), + Err(err) => return err.into(), }; let dpv = match body.0.default_package_visibility { @@ -252,9 +250,9 @@ fn update_origin( None => PackageVisibility::Public, }; - match Origin::update(&origin, dpv, &*conn) { + match Origin::update(&origin, dpv, &*conn).map_err(Error::DieselError) { Ok(_) => HttpResponse::NoContent().into(), - Err(_) => HttpResponse::InternalServerError().into(), + Err(err) => err.into(), } } @@ -262,7 +260,7 @@ fn create_keys(req: HttpRequest) -> HttpResponse { let origin = Path::::extract(&req).unwrap().into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, Some(&origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -357,7 +355,7 @@ fn upload_origin_key((req, body): (HttpRequest, String)) -> HttpRespon .into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, Some(&origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -614,7 +612,7 @@ fn do_upload_origin_secret_key(req: &HttpRequest, body: &Bytes) -> Htt let (origin, revision) = Path::<(String, String)>::extract(req).unwrap().into_inner(); // Unwrap Ok let account_id = match authorize_session(req, Some(&origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -689,7 +687,7 @@ fn list_unique_packages( let origin = Path::::extract(&req).unwrap().into_inner(); // Unwrap Ok let opt_session_id = match authorize_session(&req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; @@ -719,7 +717,7 @@ fn download_latest_origin_encryption_key(req: HttpRequest) -> HttpResp let origin = Path::::extract(&req).unwrap().into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, Some(&origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -751,7 +749,7 @@ fn invite_to_origin(req: HttpRequest) -> HttpResponse { .into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, Some(&origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -795,7 +793,7 @@ fn accept_invitation(req: HttpRequest) -> HttpResponse { .into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -826,7 +824,7 @@ fn ignore_invitation(req: HttpRequest) -> HttpResponse { .into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -857,7 +855,7 @@ fn rescind_invitation(req: HttpRequest) -> HttpResponse { .into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -948,19 +946,17 @@ fn origin_member_delete(req: HttpRequest) -> HttpResponse { .unwrap() .into_inner(); // Unwrap Ok - let account_id = match authorize_session(&req, Some(&origin)) { - Ok(id) => id, + let session = match authorize_session(&req, Some(&origin)) { + Ok(session) => session, Err(err) => return err.into(), }; - let account_name = get_session_user_name(&req, account_id); - - if !check_origin_owner(&req, account_id, &origin).unwrap_or(false) { + if !check_origin_owner(&req, session.get_id(), &origin).unwrap_or(false) { return HttpResponse::new(StatusCode::FORBIDDEN); } // Do not allow the owner to be removed which would orphan the origin - if user == account_name { + if user == session.get_name() { return HttpResponse::new(StatusCode::UNPROCESSABLE_ENTITY); } diff --git a/components/builder-api/src/server/resources/pkgs.rs b/components/builder-api/src/server/resources/pkgs.rs index 57fd71840a..f11238d5d0 100644 --- a/components/builder-api/src/server/resources/pkgs.rs +++ b/components/builder-api/src/server/resources/pkgs.rs @@ -45,7 +45,7 @@ use db::models::package::{ }; use db::models::projects::Project; -use server::authorize::{authorize_session, get_session_user_name}; +use server::authorize::authorize_session; use server::error::{Error, Result}; use server::feat; use server::framework::headers; @@ -258,7 +258,7 @@ fn download_package((qtarget, req): (Query, HttpRequest)) -> H }; let opt_session_id = match authorize_session(&req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; @@ -357,13 +357,11 @@ fn schedule_job_group((qschedule, req): (Query, HttpRequest) .unwrap() .into_inner(); // Unwrap Ok - let account_id = match authorize_session(&req, Some(&origin_name)) { - Ok(id) => id, + let session = match authorize_session(&req, Some(&origin_name)) { + Ok(session) => session, Err(err) => return err.into(), }; - let account_name = get_session_user_name(&req, account_id); - // We only support building for Linux x64 only currently if qschedule.target != "x86_64-linux" { info!("Rejecting build with target: {}", qschedule.target); @@ -378,8 +376,8 @@ fn schedule_job_group((qschedule, req): (Query, HttpRequest) request.set_origin_only(qschedule.origin_only.is_some()); request.set_package_only(qschedule.package_only.is_some()); request.set_trigger(helpers::trigger_from_request(&req)); - request.set_requester_id(account_id); - request.set_requester_name(account_name.clone()); + request.set_requester_id(session.get_id()); + request.set_requester_name(session.get_name().to_string()); match route_message::(&req, &request) { Ok(group) => { @@ -387,7 +385,7 @@ fn schedule_job_group((qschedule, req): (Query, HttpRequest) // We don't really want to abort anything just because a call to segment failed. Let's // just log it and move on. - if let Err(e) = req.state().segment.track(&account_name, &msg) { + if let Err(e) = req.state().segment.track(&session.get_name(), &msg) { warn!("Error tracking scheduling of job group in segment, {}", e); } @@ -443,7 +441,7 @@ fn get_package_channels(req: HttpRequest) -> HttpResponse { .into_inner(); // Unwrap Ok let opt_session_id = match authorize_session(&req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; @@ -482,7 +480,7 @@ fn list_package_versions(req: HttpRequest) -> HttpResponse { .into_inner(); // Unwrap Ok let opt_session_id = match authorize_session(&req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; @@ -515,7 +513,7 @@ fn search_packages((pagination, req): (Query, HttpRequest) let query = Path::::extract(&req).unwrap().into_inner(); // Unwrap Ok let opt_session_id = match authorize_session(&req, None) { - Ok(id) => Some(id as i64), + Ok(session) => Some(session.get_id() as i64), Err(_) => None, }; @@ -691,14 +689,14 @@ fn do_get_packages( pagination: &Query, ) -> Result<(Vec, i64)> { let opt_session_id = match authorize_session(req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; - let conn = req.state().db.get_conn().map_err(Error::DbError)?; - let (page, per_page) = helpers::extract_pagination_in_pages(pagination); + let conn = req.state().db.get_conn().map_err(Error::DbError)?; + let lpr = ListPackages { ident: BuilderPackageIdent(ident.clone()), visibility: helpers::visibility_for_optional_session(&req, opt_session_id, &ident.origin), @@ -814,8 +812,6 @@ fn do_upload_package_finish( return HttpResponse::with_body(StatusCode::UNPROCESSABLE_ENTITY, "ds:up:3"); } - let conn = req.state().db.get_conn().map_err(Error::DbError).unwrap(); - // Check with scheduler to ensure we don't have circular deps, if configured if feat::is_enabled(feat::Jobsrv) { if let Err(e) = check_circular_deps(&req, &ident, &target_from_artifact, &mut archive) { @@ -868,10 +864,14 @@ fn do_upload_package_finish( return HttpResponse::with_body(StatusCode::UNPROCESSABLE_ENTITY, "ds:up:6"); } - let session_id = authorize_session(&req, None).unwrap(); // Unwrap Ok - let session_name = get_session_user_name(&req, session_id); + let session = authorize_session(&req, None).unwrap(); // Unwrap Ok - package.owner_id = session_id as i64; + let conn = match req.state().db.get_conn().map_err(Error::DbError) { + Ok(conn) => conn, + Err(err) => return err.into(), + }; + + package.owner_id = session.get_id() as i64; package.origin_id = match Origin::get(&ident.origin, &*conn) { Ok(origin) => origin.id, @@ -891,7 +891,7 @@ fn do_upload_package_finish( }; // Re-create origin package as needed (eg, checksum update) - match Package::create(package.clone(), &*conn) { + match Package::create(package.clone(), &*conn).map_err(Error::DieselError) { Ok(pkg) => { if feat::is_enabled(feat::Jobsrv) { let mut job_graph_package = jobsrv::JobGraphPackageCreate::new(); @@ -907,7 +907,7 @@ fn do_upload_package_finish( } } } - Err(_) => return HttpResponse::InternalServerError().into(), + Err(err) => return err.into(), } // Schedule re-build of dependent packages (if requested) @@ -921,8 +921,8 @@ fn do_upload_package_finish( request.set_origin_only(false); request.set_package_only(false); request.set_trigger(jobsrv::JobGroupTrigger::Upload); - request.set_requester_id(session_id); - request.set_requester_name(session_name); + request.set_requester_id(session.get_id()); + request.set_requester_name(session.get_name().to_string()); match route_message::(&req, &request) { Ok(group) => debug!( @@ -989,7 +989,7 @@ fn do_get_package( ident: PackageIdent, ) -> Result { let opt_session_id = match authorize_session(req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; Counter::GetPackage.increment(); @@ -1180,13 +1180,12 @@ pub fn is_a_service(package: &Package) -> bool { m.contains("pkg_exposes") || m.contains("pkg_binds") || m.contains("pkg_exports") } -// Get platforms for a package pub fn platforms_for_package_ident( req: &HttpRequest, package: &BuilderPackageIdent, ) -> Result>> { let opt_session_id = match authorize_session(req, None) { - Ok(id) => Some(id), + Ok(session) => Some(session.get_id()), Err(_) => None, }; diff --git a/components/builder-api/src/server/resources/profile.rs b/components/builder-api/src/server/resources/profile.rs index f5f500bd7d..007f106955 100644 --- a/components/builder-api/src/server/resources/profile.rs +++ b/components/builder-api/src/server/resources/profile.rs @@ -69,7 +69,7 @@ pub fn do_get_access_tokens( // fn get_account(req: HttpRequest) -> HttpResponse { let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(_err) => return HttpResponse::new(StatusCode::UNAUTHORIZED), }; @@ -86,7 +86,7 @@ fn get_account(req: HttpRequest) -> HttpResponse { fn get_access_tokens(req: HttpRequest) -> HttpResponse { let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -106,7 +106,7 @@ fn get_access_tokens(req: HttpRequest) -> HttpResponse { fn generate_access_token(req: HttpRequest) -> HttpResponse { let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -160,7 +160,7 @@ fn revoke_access_token(req: HttpRequest) -> HttpResponse { }; let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -188,7 +188,7 @@ fn revoke_access_token(req: HttpRequest) -> HttpResponse { fn update_account((req, body): (HttpRequest, Json)) -> HttpResponse { let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(_err) => return HttpResponse::new(StatusCode::UNAUTHORIZED), }; diff --git a/components/builder-api/src/server/resources/projects.rs b/components/builder-api/src/server/resources/projects.rs index b2d93927c4..cd1669cd61 100644 --- a/components/builder-api/src/server/resources/projects.rs +++ b/components/builder-api/src/server/resources/projects.rs @@ -109,7 +109,7 @@ fn create_project((req, body): (HttpRequest, Json)) } let account_id = match authorize_session(&req, Some(&body.origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -265,7 +265,7 @@ fn update_project((req, body): (HttpRequest, Json)) .into_inner(); // Unwrap Ok let account_id = match authorize_session(&req, Some(&origin)) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; diff --git a/components/builder-api/src/server/resources/user.rs b/components/builder-api/src/server/resources/user.rs index ca93312265..b0e33ba94c 100644 --- a/components/builder-api/src/server/resources/user.rs +++ b/components/builder-api/src/server/resources/user.rs @@ -37,7 +37,7 @@ impl User { // fn get_invitations(req: HttpRequest) -> HttpResponse { let account_id = match authorize_session(&req, None) { - Ok(id) => id, + Ok(session) => session.get_id(), Err(err) => return err.into(), }; @@ -54,7 +54,7 @@ fn get_invitations(req: HttpRequest) -> HttpResponse { fn get_origins(req: HttpRequest) -> HttpResponse { let account_id = match authorize_session(&req, None) { - Ok(id) => id as i64, + Ok(session) => session.get_id() as i64, Err(err) => return err.into(), }; diff --git a/components/builder-api/src/server/services/memcache.rs b/components/builder-api/src/server/services/memcache.rs index 7e58b5a082..6f3bc79745 100644 --- a/components/builder-api/src/server/services/memcache.rs +++ b/components/builder-api/src/server/services/memcache.rs @@ -145,6 +145,25 @@ impl MemcacheClient { }; } + pub fn set_origin_member(&mut self, origin: &str, account_id: u64, val: bool) { + let key = format!("member:{}/{}", origin, account_id); + + match self.cli.set(&key, val, self.ttl * 60) { + Ok(_) => trace!( + "Saved origin membership {}/{} to memcached!", + origin, + account_id + ), + Err(e) => warn!("Failed to save origin membership to memcached: {}", e), + } + } + + pub fn get_origin_member(&mut self, origin: &str, account_id: u64) -> Option { + let key = format!("member:{}/{}", origin, account_id); + + self.get_bool(&key) + } + fn package_namespace(&mut self, origin: &str, name: &str) -> String { self.get_namespace(&package_ns_key(origin, name)) } @@ -189,6 +208,16 @@ impl MemcacheClient { } } } + + fn get_bool(&mut self, key: &str) -> Option { + match self.cli.get(key) { + Ok(val) => val, + Err(e) => { + warn!("Error getting key {}: {:?}", key, e); + None + } + } + } } fn package_ns_key(origin: &str, name: &str) -> String { From 826be65f46f6ffa40fca227eda9689f262e8d949 Mon Sep 17 00:00:00 2001 From: Salim Alam Date: Fri, 2 Nov 2018 23:18:43 -0700 Subject: [PATCH 2/3] Add DBCall counter metrics to model Signed-off-by: Salim Alam --- Cargo.lock | 1 + components/builder-db/Cargo.toml | 3 ++ components/builder-db/src/lib.rs | 2 + components/builder-db/src/models/account.rs | 11 ++++++ components/builder-db/src/models/channel.rs | 16 ++++++++ .../builder-db/src/models/integration.rs | 8 ++++ .../builder-db/src/models/invitations.rs | 9 +++++ components/builder-db/src/models/keys.rs | 15 ++++++++ components/builder-db/src/models/origin.rs | 10 +++++ components/builder-db/src/models/package.rs | 38 ++++++++++++++----- .../src/models/project_integration.rs | 7 ++++ components/builder-db/src/models/projects.rs | 8 ++++ components/builder-db/src/models/secrets.rs | 7 ++++ 13 files changed, 125 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 67396cd652..f3d2ed4e58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1015,6 +1015,7 @@ dependencies = [ name = "habitat_builder_db" version = "0.0.0" dependencies = [ + "builder_core 0.0.0", "chrono 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "clippy 0.0.302 (registry+https://github.com/rust-lang/crates.io-index)", "diesel 1.3.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/builder-db/Cargo.toml b/components/builder-db/Cargo.toml index d86dce855d..36958dfcd2 100644 --- a/components/builder-db/Cargo.toml +++ b/components/builder-db/Cargo.toml @@ -35,3 +35,6 @@ git = "https://github.com/habitat-sh/core.git" [dependencies.habitat_net] path = "../net" + +[dependencies.builder_core] +path = "../builder-core" \ No newline at end of file diff --git a/components/builder-db/src/lib.rs b/components/builder-db/src/lib.rs index 6f1449667a..45282678c3 100644 --- a/components/builder-db/src/lib.rs +++ b/components/builder-db/src/lib.rs @@ -22,6 +22,7 @@ extern crate diesel_derive_enum; extern crate diesel_full_text_search; #[macro_use] extern crate diesel_migrations; +extern crate builder_core as bldr_core; extern crate fallible_iterator; extern crate fnv; extern crate habitat_builder_protocol as protocol; @@ -50,6 +51,7 @@ extern crate url; pub mod config; pub mod diesel_pool; pub mod error; +pub mod metrics; pub mod migration; pub mod models; pub mod pool; diff --git a/components/builder-db/src/models/account.rs b/components/builder-db/src/models/account.rs index 90d1e2a65f..5c00702fe2 100644 --- a/components/builder-db/src/models/account.rs +++ b/components/builder-db/src/models/account.rs @@ -7,6 +7,9 @@ use diesel::sql_types::{BigInt, Text}; use diesel::RunQueryDsl; use schema::account::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Identifiable, Debug, Serialize, QueryableByName)] #[table_name = "accounts"] pub struct Account { @@ -38,18 +41,21 @@ pub struct NewAccount<'a> { impl Account { pub fn get(name: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM get_account_by_name_v1($1)") .bind::(name) .get_result(conn) } pub fn get_by_id(id: u64, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM get_account_by_id_v1($1)") .bind::(id as i64) .get_result(conn) } pub fn create(account: &NewAccount, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM select_or_insert_account_v1($1, $2)") .bind::(account.name) .bind::(account.email) @@ -57,6 +63,7 @@ impl Account { } pub fn find_or_create(name: &str, email: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM select_or_insert_account_v1($1, $2)") .bind::(name) .bind::(email) @@ -64,6 +71,7 @@ impl Account { } pub fn update(id: u64, email: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT update_account_v1($1, $2)") .bind::(id as i64) .bind::(email) @@ -80,12 +88,14 @@ pub struct NewAccountToken<'a> { impl AccountToken { pub fn list(account_id: u64, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM get_account_tokens_v1($1)") .bind::(account_id as i64) .get_results(conn) } pub fn create(req: &NewAccountToken, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM insert_account_token_v1($1, $2)") .bind::(req.account_id) .bind::(req.token) @@ -93,6 +103,7 @@ impl AccountToken { } pub fn delete(id: u64, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT revoke_account_token_v1($1)") .bind::(id as i64) .execute(conn) diff --git a/components/builder-db/src/models/channel.rs b/components/builder-db/src/models/channel.rs index 673b2eae05..ffe43fc118 100644 --- a/components/builder-db/src/models/channel.rs +++ b/components/builder-db/src/models/channel.rs @@ -12,6 +12,9 @@ use models::pagination::Paginate; use protocol::jobsrv::JobGroupTrigger; use schema::channel::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName, Queryable, Identifiable)] #[table_name = "origin_channels"] pub struct Channel { @@ -76,6 +79,7 @@ pub struct DemotePackages { impl Channel { pub fn list(channel: ListChannels, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_channels_for_origin_v3($1, $2)") .bind::(channel.origin) .bind::(channel.include_sandbox_channels) @@ -83,6 +87,7 @@ impl Channel { } pub fn get(channel: GetChannel, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_channel_v1($1, $2)") .bind::(channel.origin) .bind::(channel.channel) @@ -90,6 +95,7 @@ impl Channel { } pub fn create(channel: CreateChannel, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from insert_origin_channel_v2($1, $2, $3)") .bind::(channel.origin) .bind::(channel.owner_id) @@ -98,6 +104,7 @@ impl Channel { } pub fn delete(channel: DeleteChannel, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from delete_origin_channel_v2($1, $2)") .bind::(channel.channel) .bind::(channel.origin) @@ -105,6 +112,7 @@ impl Channel { } pub fn get_latest_package(req: GetLatestPackage, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); let ident = req.ident; diesel::sql_query("select * from get_origin_channel_package_latest_v8($1, $2, $3, $4, $5)") .bind::(&ident.origin) @@ -119,6 +127,8 @@ impl Channel { lcp: ListChannelPackages, conn: &PgConnection, ) -> QueryResult<(Vec, i64)> { + Counter::DBCall.increment(); + use schema::channel::{origin_channel_packages, origin_channels}; use schema::origin::origins; use schema::package::origin_packages; @@ -139,6 +149,7 @@ impl Channel { } pub fn promote_packages(req: PromotePackages, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from promote_origin_package_group_v1($1, $2)") .bind::(req.channel_id) .bind::, _>(req.pkg_ids) @@ -146,6 +157,7 @@ impl Channel { } pub fn demote_packages(req: DemotePackages, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from demote_origin_package_group_v1($1, $2)") .bind::(req.channel_id) .bind::, _>(req.pkg_ids) @@ -189,6 +201,7 @@ pub struct PackageChannelAudit<'a> { impl<'a> PackageChannelAudit<'a> { pub fn audit(pca: PackageChannelAudit, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from add_audit_package_entry_v3($1, $2, $3, $4, $5, $6, $7)") .bind::(pca.origin) .bind::(pca.ident.to_string()) @@ -215,6 +228,7 @@ pub struct PackageGroupChannelAudit<'a> { impl<'a> PackageGroupChannelAudit<'a> { pub fn audit(req: PackageGroupChannelAudit, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query( "select * from add_audit_package_group_entry_v2($1, $2, $3, $4, $5, $6, $7, $8)", ).bind::(req.origin) @@ -249,6 +263,7 @@ pub struct OriginChannelDemote { impl OriginChannelPackage { pub fn promote(package: OriginChannelPromote, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from promote_origin_package_v3($1, $2, $3)") .bind::(package.origin) .bind::(package.ident.to_string()) @@ -256,6 +271,7 @@ impl OriginChannelPackage { .execute(conn) } pub fn demote(package: OriginChannelDemote, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from demote_origin_package_v3($1, $2, $3)") .bind::(package.origin) .bind::(package.ident.to_string()) diff --git a/components/builder-db/src/models/integration.rs b/components/builder-db/src/models/integration.rs index 6a29d6c579..36517c5b65 100644 --- a/components/builder-db/src/models/integration.rs +++ b/components/builder-db/src/models/integration.rs @@ -7,6 +7,9 @@ use diesel::sql_types::Text; use diesel::RunQueryDsl; use schema::integration::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origin_integrations"] pub struct OriginIntegration { @@ -31,6 +34,7 @@ pub struct NewOriginIntegration<'a> { impl OriginIntegration { pub fn create(req: &NewOriginIntegration, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from upsert_origin_integration_v1($1, $2, $3, $4)") .bind::(req.origin) .bind::(req.integration) @@ -45,6 +49,7 @@ impl OriginIntegration { name: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_integration_v1($1, $2, $3)") .bind::(origin) .bind::(integration) @@ -58,6 +63,7 @@ impl OriginIntegration { name: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from delete_origin_integration_v1($1, $2, $3)") .bind::(origin) .bind::(integration) @@ -70,6 +76,7 @@ impl OriginIntegration { integration: &str, conn: &PgConnection, ) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_integrations_v1($1, $2)") .bind::(origin) .bind::(integration) @@ -80,6 +87,7 @@ impl OriginIntegration { origin: &str, conn: &PgConnection, ) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_integrations_for_origin_v1($1)") .bind::(origin) .get_results(conn) diff --git a/components/builder-db/src/models/invitations.rs b/components/builder-db/src/models/invitations.rs index e419334136..ec62b5d32f 100644 --- a/components/builder-db/src/models/invitations.rs +++ b/components/builder-db/src/models/invitations.rs @@ -7,6 +7,9 @@ use diesel::sql_types::{BigInt, Bool, Text}; use diesel::RunQueryDsl; use schema::invitation::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origin_invitations"] pub struct OriginInvitation { @@ -37,6 +40,7 @@ pub struct NewOriginInvitation<'a> { impl OriginInvitation { pub fn create(req: &NewOriginInvitation, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from insert_origin_invitation_v1($1, $2, $3, $4, $5)") .bind::(req.origin_id) .bind::(req.origin_name) @@ -50,6 +54,7 @@ impl OriginInvitation { origin_id: u64, conn: &PgConnection, ) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_invitations_for_origin_v1($1)") .bind::(origin_id as i64) .get_results(conn) @@ -59,12 +64,14 @@ impl OriginInvitation { owner_id: u64, conn: &PgConnection, ) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_invitations_for_account_v1($1)") .bind::(owner_id as i64) .get_results(conn) } pub fn accept(invite_id: u64, ignore: bool, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from accept_origin_invitation_v1($1, $2)") .bind::(invite_id as i64) .bind::(ignore) @@ -72,6 +79,7 @@ impl OriginInvitation { } pub fn ignore(invite_id: u64, account_id: u64, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from ignore_origin_invitation_v1($1, $2)") .bind::(invite_id as i64) .bind::(account_id as i64) @@ -79,6 +87,7 @@ impl OriginInvitation { } pub fn rescind(invite_id: u64, account_id: u64, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from rescind_origin_invitation_v1($1, $2)") .bind::(invite_id as i64) .bind::(account_id as i64) diff --git a/components/builder-db/src/models/keys.rs b/components/builder-db/src/models/keys.rs index ef404a0567..8003c1b579 100644 --- a/components/builder-db/src/models/keys.rs +++ b/components/builder-db/src/models/keys.rs @@ -7,6 +7,9 @@ use diesel::sql_types::{BigInt, Binary, Text}; use diesel::RunQueryDsl; use schema::key::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origin_public_encryption_keys"] pub struct OriginPublicEncryptionKey { @@ -121,6 +124,7 @@ impl OriginPublicEncryptionKey { revision: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_public_encryption_key_v1($1, $2)") .bind::(origin) .bind::(revision) @@ -131,6 +135,7 @@ impl OriginPublicEncryptionKey { req: &NewOriginPublicEncryptionKey, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); let full_name = format!("{}-{}", req.name, req.revision); diesel::sql_query( "select * from insert_origin_public_encryption_key_v1($1, $2, $3, $4, $5, $6)", @@ -144,12 +149,14 @@ impl OriginPublicEncryptionKey { } pub fn latest(origin: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_public_encryption_key_latest_v1($1)") .bind::(origin) .get_result(conn) } pub fn list(origin: &str, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_public_encryption_keys_for_origin_v1($1)") .bind::(origin) .get_results(conn) @@ -158,6 +165,7 @@ impl OriginPublicEncryptionKey { impl OriginPrivateEncryptionKey { pub fn get(origin: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_private_encryption_key_v1($1)") .bind::(origin) .get_result(conn) @@ -167,6 +175,7 @@ impl OriginPrivateEncryptionKey { req: &NewOriginPrivateEncryptionKey, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); let full_name = format!("{}-{}", req.name, req.revision); diesel::sql_query( "select * from insert_origin_private_encryption_key_v1($1, $2, $3, $4, $5, $6)", @@ -186,6 +195,7 @@ impl OriginPublicSigningKey { revision: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_public_key_v1($1, $2)") .bind::(origin) .bind::(revision) @@ -196,6 +206,7 @@ impl OriginPublicSigningKey { req: &NewOriginPublicSigningKey, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); let full_name = format!("{}-{}", req.name, req.revision); diesel::sql_query("select * from insert_origin_public_key_v1($1, $2, $3, $4, $5, $6)") .bind::(req.origin_id) @@ -208,12 +219,14 @@ impl OriginPublicSigningKey { } pub fn latest(origin: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_public_key_latest_v1($1)") .bind::(origin) .get_result(conn) } pub fn list(origin_id: u64, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_public_keys_for_origin_v1($1)") .bind::(origin_id as i64) .get_results(conn) @@ -222,6 +235,7 @@ impl OriginPublicSigningKey { impl OriginPrivateSigningKey { pub fn get(origin: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_secret_key_v1($1)") .bind::(origin) .get_result(conn) @@ -231,6 +245,7 @@ impl OriginPrivateSigningKey { req: &NewOriginPrivateSigningKey, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); let full_name = format!("{}-{}", req.name, req.revision); diesel::sql_query("select * from insert_origin_secret_key_v1($1, $2, $3, $4, $5, $6)") .bind::(req.origin_id) diff --git a/components/builder-db/src/models/origin.rs b/components/builder-db/src/models/origin.rs index 977deb5761..70cc00bdd2 100644 --- a/components/builder-db/src/models/origin.rs +++ b/components/builder-db/src/models/origin.rs @@ -14,6 +14,9 @@ use protocol::originsrv; use schema::member::*; use schema::origin::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origins"] pub struct Origin { @@ -78,6 +81,7 @@ pub struct NewOrigin<'a> { impl Origin { pub fn get(origin: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query( "select * from origins_with_secret_key_full_name_v2 where name = $1 limit 1", ).bind::(origin) @@ -85,12 +89,14 @@ impl Origin { } pub fn list(owner_id: i64, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from my_origins_with_stats_v2($1)") .bind::(owner_id) .get_results(conn) } pub fn create(req: &NewOrigin, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from insert_origin_v3($1, $2, $3, $4)") .bind::(req.name) .bind::(req.owner_id) @@ -100,6 +106,7 @@ impl Origin { } pub fn update(name: &str, dpv: PackageVisibility, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from update_origin_v2($1, $2)") .bind::(name) .bind::(dpv) @@ -111,6 +118,7 @@ impl Origin { account_id: u64, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from check_account_in_origin_members_v1($1, $2)") .bind::(origin) .bind::(account_id as i64) @@ -124,6 +132,7 @@ impl OriginMember { use schema::member::origin_members; use schema::origin::origins; + Counter::DBCall.increment(); origin_members::table .select(origin_members::table::all_columns()) .inner_join(origins::table) @@ -133,6 +142,7 @@ impl OriginMember { } pub fn delete(origin_id: u64, account_name: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from delete_origin_member_v1($1, $2)") .bind::(origin_id as i64) .bind::(account_name) diff --git a/components/builder-db/src/models/package.rs b/components/builder-db/src/models/package.rs index e7ecd3961b..007fbc8d61 100644 --- a/components/builder-db/src/models/package.rs +++ b/components/builder-db/src/models/package.rs @@ -27,6 +27,9 @@ use models::channel::Channel; use models::pagination::*; use schema::package::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName, Queryable, Clone, Identifiable)] #[table_name = "origin_packages"] pub struct Package { @@ -212,10 +215,7 @@ impl PackageVisibility { } pub fn private() -> Vec { - vec![ - PackageVisibility::Private, - PackageVisibility::Hidden, - ] + vec![PackageVisibility::Private, PackageVisibility::Hidden] } } @@ -223,6 +223,7 @@ impl Package { pub fn get(req: GetPackage, conn: &PgConnection) -> QueryResult { use schema::package::origin_packages::dsl::*; + Counter::DBCall.increment(); Self::all() .filter(ident.eq(req.ident)) .filter(visibility.eq(any(req.visibility))) @@ -236,12 +237,14 @@ impl Package { ) -> QueryResult> { use schema::package::origin_packages::dsl::*; + Counter::DBCall.increment(); Self::all() .filter(ident_array.contains(req_ident.parts())) .get_results(conn) } pub fn get_latest(req: GetLatestPackage, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_package_latest_v7($1, $2, $3)") .bind::, _>(req.ident.parts()) .bind::(req.target) @@ -250,6 +253,7 @@ impl Package { } pub fn create(package: NewPackage, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM insert_origin_package_v5($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)") .bind::(package.origin_id) .bind::(package.owner_id) @@ -273,6 +277,7 @@ impl Package { ) -> QueryResult { use schema::package::origin_packages::dsl::*; + Counter::DBCall.increment(); diesel::update(origin_packages.filter(ident.eq(idt))) .set(visibility.eq(vis)) .execute(conn) @@ -282,6 +287,7 @@ impl Package { req: UpdatePackageVisibility, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from update_package_visibility_in_bulk_v2($1, $2)") .bind::(req.visibility) .bind::, _>(req.ids) @@ -296,6 +302,7 @@ impl Package { ident, ident_array, origin_packages, visibility, }; + Counter::DBCall.increment(); origin_packages .select(ident) .filter(ident_array.contains(pl.ident.parts())) @@ -311,6 +318,8 @@ impl Package { conn: &PgConnection, ) -> QueryResult<(Vec, i64)> { use schema::package::origin_packages::dsl::{ident_array, origin_packages, visibility}; + + Counter::DBCall.increment(); origin_packages .select(sql("concat_ws('/', ident_array[1], ident_array[2])")) .filter(ident_array.contains(pl.ident.parts())) @@ -330,6 +339,7 @@ impl Package { use schema::origin::origins; use schema::package::origin_packages; + Counter::DBCall.increment(); origin_packages::table .inner_join(origins::table) .select(sql("concat_ws('/', origins.name, origin_packages.name)")) @@ -353,6 +363,7 @@ impl Package { ident as package_ident, origin_packages, visibility as package_visibility, }; + Counter::DBCall.increment(); origin_packages .inner_join(origin_channel_packages.inner_join(origin_channels)) .select(origin_channels::all_columns()) @@ -367,6 +378,7 @@ impl Package { visibility: Vec, conn: &PgConnection, ) -> QueryResult> { + Counter::DBCall.increment(); // So many regrets with this query diesel::sql_query( "select $1 as origin, $2 as name, version, release_count, latest, string_to_array(platforms, ',') as platforms from get_origin_package_versions_for_origin_v8($1, $2, $3)", @@ -383,6 +395,7 @@ impl Package { use schema::origin::origins; use schema::package::origin_packages; + Counter::DBCall.increment(); let mut query = origin_packages::table .inner_join(origins::table) .select(origin_packages::ident) @@ -391,10 +404,12 @@ impl Package { .into_boxed(); if let Some(session_id) = sp.account_id { - query = query - .filter(origin_packages::visibility.eq(any(PackageVisibility::private())) + query = query.filter( + origin_packages::visibility + .eq(any(PackageVisibility::private())) .and(origins::owner_id.eq(session_id)) - .or(origin_packages::visibility.eq(PackageVisibility::Public))); + .or(origin_packages::visibility.eq(PackageVisibility::Public)), + ); } else { query = query.filter(origin_packages::visibility.eq(PackageVisibility::Public)); } @@ -410,6 +425,7 @@ impl Package { sp: SearchPackages, conn: &PgConnection, ) -> QueryResult<(Vec, i64)> { + Counter::DBCall.increment(); use schema::origin::origins; use schema::package::origin_packages; @@ -421,10 +437,12 @@ impl Package { .into_boxed(); if let Some(session_id) = sp.account_id { - query = query - .filter(origin_packages::visibility.eq(any(PackageVisibility::private())) + query = query.filter( + origin_packages::visibility + .eq(any(PackageVisibility::private())) .and(origins::owner_id.eq(session_id)) - .or(origin_packages::visibility.eq(PackageVisibility::Public))); + .or(origin_packages::visibility.eq(PackageVisibility::Public)), + ); } else { query = query.filter(origin_packages::visibility.eq(PackageVisibility::Public)); } diff --git a/components/builder-db/src/models/project_integration.rs b/components/builder-db/src/models/project_integration.rs index 050ac00e7a..d54d9bf7a5 100644 --- a/components/builder-db/src/models/project_integration.rs +++ b/components/builder-db/src/models/project_integration.rs @@ -8,6 +8,9 @@ use diesel::RunQueryDsl; use protocol::originsrv; use schema::project_integration::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origin_project_integrations"] pub struct ProjectIntegration { @@ -40,6 +43,7 @@ impl ProjectIntegration { integration: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_project_integrations_v2($1, $2, $3)") .bind::(origin) .bind::(name) @@ -52,6 +56,7 @@ impl ProjectIntegration { name: &str, conn: &PgConnection, ) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_project_integrations_for_project_v2($1, $2)") .bind::(origin) .bind::(name) @@ -64,6 +69,7 @@ impl ProjectIntegration { integration: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from delete_origin_project_integration_v1($1, $2, $3)") .bind::(origin) .bind::(name) @@ -72,6 +78,7 @@ impl ProjectIntegration { } pub fn create(req: NewProjectIntegration, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("SELECT * FROM upsert_origin_project_integration_v3($1, $2, $3, $4)") .bind::(req.origin) .bind::(req.name) diff --git a/components/builder-db/src/models/projects.rs b/components/builder-db/src/models/projects.rs index aa73d85044..daa165f4ce 100644 --- a/components/builder-db/src/models/projects.rs +++ b/components/builder-db/src/models/projects.rs @@ -9,6 +9,9 @@ use models::package::{PackageVisibility, PackageVisibilityMapping}; use protocol::originsrv; use schema::project::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origin_projects"] pub struct Project { @@ -59,18 +62,21 @@ pub struct UpdateProject<'a> { impl Project { pub fn get(name: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_project_v1($1)") .bind::(name) .get_result(conn) } pub fn delete(name: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from delete_origin_project_v1($1)") .bind::(name) .execute(conn) } pub fn create(project: &NewProject, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query( "SELECT * FROM insert_origin_project_v6($1, $2, $3, $4, $5, $6, $7, $8, $9)", ).bind::(project.origin_name) @@ -86,6 +92,7 @@ impl Project { } pub fn update(project: &UpdateProject, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query( "SELECT * FROM update_origin_project_v5($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)", ).bind::(project.id) @@ -102,6 +109,7 @@ impl Project { } pub fn list(origin: &str, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_project_list_v2($1)") .bind::(origin) .get_results(conn) diff --git a/components/builder-db/src/models/secrets.rs b/components/builder-db/src/models/secrets.rs index 8456972000..e298536d24 100644 --- a/components/builder-db/src/models/secrets.rs +++ b/components/builder-db/src/models/secrets.rs @@ -7,6 +7,9 @@ use diesel::sql_types::{BigInt, Text}; use diesel::RunQueryDsl; use schema::secrets::*; +use bldr_core::metrics::CounterMetric; +use metrics::Counter; + #[derive(Debug, Serialize, Deserialize, QueryableByName)] #[table_name = "origin_secrets"] pub struct OriginSecret { @@ -29,6 +32,7 @@ impl OriginSecret { value: &str, conn: &PgConnection, ) -> QueryResult { + Counter::DBCall.increment(); // TODO FIX: We're missing setting the account id here diesel::sql_query("select * from insert_origin_secret_v1($1, $2, $3)") .bind::(origin_id) @@ -38,6 +42,7 @@ impl OriginSecret { } pub fn get(origin_id: i64, name: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_secret_v1($1, $2)") .bind::(origin_id) .bind::(name) @@ -45,6 +50,7 @@ impl OriginSecret { } pub fn delete(origin_id: i64, name: &str, conn: &PgConnection) -> QueryResult { + Counter::DBCall.increment(); diesel::sql_query("select * from delete_origin_secret_v1($1, $2)") .bind::(origin_id) .bind::(name) @@ -52,6 +58,7 @@ impl OriginSecret { } pub fn list(origin_id: i64, conn: &PgConnection) -> QueryResult> { + Counter::DBCall.increment(); diesel::sql_query("select * from get_origin_secrets_for_origin_v1($1)") .bind::(origin_id) .get_results(conn) From 342cdcda5025a4f516fddbb6266c036c67cb8adf Mon Sep 17 00:00:00 2001 From: Salim Alam Date: Fri, 2 Nov 2018 23:34:43 -0700 Subject: [PATCH 3/3] Update memcache tracing Signed-off-by: Salim Alam --- components/builder-api/src/server/authorize.rs | 11 ++++++++++- .../builder-api/src/server/resources/channels.rs | 4 ++-- .../builder-api/src/server/services/memcache.rs | 6 ++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/components/builder-api/src/server/authorize.rs b/components/builder-api/src/server/authorize.rs index 3142d40943..118ca1fc3d 100644 --- a/components/builder-api/src/server/authorize.rs +++ b/components/builder-api/src/server/authorize.rs @@ -45,13 +45,22 @@ pub fn authorize_session( match memcache.get_origin_member(origin, session.get_id()) { Some(val) => { + trace!( + "Origin membership {} {} Cache Hit!", + origin, + session.get_id() + ); if val { return Ok(session); } else { return Err(Error::Authorization); } } - None => (), + None => trace!( + "Origin membership {} {} Cache Miss!", + origin, + session.get_id() + ), } match check_origin_member(req, origin, session.get_id()) { diff --git a/components/builder-api/src/server/resources/channels.rs b/components/builder-api/src/server/resources/channels.rs index 9d8ccde86e..372103740c 100644 --- a/components/builder-api/src/server/resources/channels.rs +++ b/components/builder-api/src/server/resources/channels.rs @@ -509,11 +509,11 @@ fn do_get_channel_package( let mut memcache = req.state().memcache.borrow_mut(); match memcache.get_package(req_ident.clone().into(), &channel, &target) { Some(pkg_json) => { - trace!("Cache Hit!"); + trace!("Channel package {} {} Cache Hit!", channel, ident); return Ok(pkg_json); } None => { - trace!("Cache Miss!"); + trace!("Channel package {} {} Cache Miss!", channel, ident); } }; } diff --git a/components/builder-api/src/server/services/memcache.rs b/components/builder-api/src/server/services/memcache.rs index 6f3bc79745..f3a162c6ac 100644 --- a/components/builder-api/src/server/services/memcache.rs +++ b/components/builder-api/src/server/services/memcache.rs @@ -159,6 +159,12 @@ impl MemcacheClient { } pub fn get_origin_member(&mut self, origin: &str, account_id: u64) -> Option { + trace!( + "Getting origin membership for {} {} from memcached", + origin, + account_id + ); + let key = format!("member:{}/{}", origin, account_id); self.get_bool(&key)