Skip to content

Commit

Permalink
perf: Port get_bsos' pagination optimization
Browse files Browse the repository at this point in the history 
Fix mozilla-services#53
  • Loading branch information
@fzzzy
fzzzy committed Jan 24, 2020
1 parent cf31fde commit 9266f75
Show file tree
Hide file tree
Showing 9 changed files with 220 additions and 65 deletions.
53 changes: 31 additions & 22 deletions db-tests/src/db_tests.rs
View file
Expand Up @@ -128,12 +128,12 @@ async fn get_bsos_limit_offset() -> Result<()> {
0,
Sorting::Index,
0,
0,
&"0".to_owned(),
))
.compat()
.await?;
assert!(bsos.items.is_empty());
assert_eq!(bsos.offset, Some(0));
assert_eq!(bsos.offset, Some("0".to_string()));

let bsos = db
.get_bsos(gbsos(
Expand All @@ -144,7 +144,7 @@ async fn get_bsos_limit_offset() -> Result<()> {
0,
Sorting::Index,
-1,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -153,7 +153,7 @@ async fn get_bsos_limit_offset() -> Result<()> {

let newer = 0;
let limit = 5;
let offset = 0;
let offset = "0".to_owned();
// XXX: validation?
/*
let bsos = db.get_bsos_sync(gbsos(uid, coll, &[], MAX_TIMESTAMP, 0, Sorting::Index, -1, 0))?;
Expand All @@ -169,12 +169,17 @@ async fn get_bsos_limit_offset() -> Result<()> {
newer,
Sorting::Newest,
limit,
offset,
&offset,
))
.compat()
.await?;
assert_eq!(bsos.items.len(), 5 as usize);
assert_eq!(bsos.offset, Some(5));
if let Some(ref offset) = bsos.offset {
if offset.chars().position(|c| c == ':').is_none() {
assert_eq!(offset, &"5".to_string());
}
}

assert_eq!(bsos.items[0].id, "11");
assert_eq!(bsos.items[4].id, "7");

Expand All @@ -185,14 +190,18 @@ async fn get_bsos_limit_offset() -> Result<()> {
&[],
MAX_TIMESTAMP,
newer,
Sorting::Index,
Sorting::Newest,
limit,
bsos.offset.unwrap(),
&bsos.offset.unwrap(),
))
.compat()
.await?;
assert_eq!(bsos2.items.len(), 5 as usize);
assert_eq!(bsos2.offset, Some(10));
if let Some(ref offset) = bsos2.offset {
if offset.chars().position(|c| c == ':').is_none() {
assert_eq!(offset, &"10".to_owned());
}
}
assert_eq!(bsos2.items[0].id, "6");
assert_eq!(bsos2.items[4].id, "2");

Expand All @@ -203,9 +212,9 @@ async fn get_bsos_limit_offset() -> Result<()> {
&[],
MAX_TIMESTAMP,
newer,
Sorting::Index,
Sorting::Newest,
limit,
bsos2.offset.unwrap(),
&bsos2.offset.unwrap(),
))
.compat()
.await?;
Expand Down Expand Up @@ -245,7 +254,7 @@ async fn get_bsos_newer() -> Result<()> {
timestamp as u64 - 30,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -263,7 +272,7 @@ async fn get_bsos_newer() -> Result<()> {
timestamp as u64 - 20,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -280,7 +289,7 @@ async fn get_bsos_newer() -> Result<()> {
timestamp as u64 - 10,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -296,7 +305,7 @@ async fn get_bsos_newer() -> Result<()> {
timestamp as u64,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand Down Expand Up @@ -334,7 +343,7 @@ async fn get_bsos_sort() -> Result<()> {
0,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -352,7 +361,7 @@ async fn get_bsos_sort() -> Result<()> {
0,
Sorting::Oldest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -370,7 +379,7 @@ async fn get_bsos_sort() -> Result<()> {
0,
Sorting::Index,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand Down Expand Up @@ -843,7 +852,7 @@ async fn get_bsos() -> Result<()> {
0,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -858,7 +867,7 @@ async fn get_bsos() -> Result<()> {
0,
Sorting::Newest,
10,
0,
&"0".to_owned(),
))
.compat()
.await?;
Expand All @@ -876,12 +885,12 @@ async fn get_bsos() -> Result<()> {
0,
Sorting::Index,
2,
0,
&"0".to_owned(),
))
.compat()
.await?;
assert_eq!(bsos.items.len(), 2);
assert_eq!(bsos.offset, Some(2));
assert_eq!(bsos.offset, Some("2".to_string()));
assert_eq!(bsos.items[0].id, "b2");
assert_eq!(bsos.items[1].id, "b1");
Ok(())
Expand Down
8 changes: 4 additions & 4 deletions db-tests/src/support.rs
View file
@@ -1,12 +1,12 @@
use env_logger;
use futures::compat::Future01CompatExt;

use std::str::FromStr;
use syncstorage::{
db::{params, util::SyncTimestamp, Db, Sorting},
error::ApiError,
server::metrics,
settings::{Secrets, ServerLimits, Settings},
web::extractors::{BsoQueryParams, HawkIdentifier},
web::extractors::{BsoQueryParams, HawkIdentifier, Offset},
};

pub type Result<T> = std::result::Result<T, ApiError>;
Expand Down Expand Up @@ -95,7 +95,7 @@ pub fn gbsos(
newer: u64,
sort: Sorting,
limit: i64,
offset: i64,
offset: &str,
) -> params::GetBsos {
params::GetBsos {
user_id: hid(user_id),
Expand All @@ -106,7 +106,7 @@ pub fn gbsos(
newer: Some(SyncTimestamp::from_milliseconds(newer)),
sort,
limit: Some(limit as u32),
offset: Some(offset as u64),
offset: Some(Offset::from_str(offset).unwrap_or_default()),
full: true,
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/db/mod.rs
View file
Expand Up @@ -228,7 +228,7 @@ impl Clone for Box<dyn Db> {
}
}

#[derive(Debug, Deserialize, Clone, PartialEq)]
#[derive(Debug, Deserialize, Clone, PartialEq, Copy)]
#[serde(rename_all = "lowercase")]
pub enum Sorting {
None,
Expand Down
17 changes: 9 additions & 8 deletions src/db/mysql/models.rs
View file
Expand Up @@ -495,11 +495,12 @@ impl MysqlDb {
// match the query conditions
query = query.limit(if limit >= 0 { limit + 1 } else { limit });

let offset = offset.unwrap_or(0) as i64;
if offset != 0 {
let numeric_offset = offset.map_or(0, |offset| offset.offset);

if numeric_offset != 0 {
// XXX: copy over this optimization:
// https://github.com/mozilla-services/server-syncstorage/blob/a0f8117/syncstorage/storage/sql/__init__.py#L404
query = query.offset(offset);
query = query.offset(numeric_offset);
}
let mut bsos = query.load::<results::GetBso>(&self.conn)?;

Expand All @@ -510,7 +511,7 @@ impl MysqlDb {

let next_offset = if limit >= 0 && bsos.len() > limit as usize {
bsos.pop();
Some(limit + offset)
Some((limit + numeric_offset).to_string())
} else {
None
};
Expand Down Expand Up @@ -564,11 +565,11 @@ impl MysqlDb {
// match the query conditions
query = query.limit(if limit >= 0 { limit + 1 } else { limit });

let offset = offset.unwrap_or(0) as i64;
if offset != 0 {
let numeric_offset = offset.map_or(0, |offset| offset.offset);
if numeric_offset != 0 {
// XXX: copy over this optimization:
// https://github.com/mozilla-services/server-syncstorage/blob/a0f8117/syncstorage/storage/sql/__init__.py#L404
query = query.offset(offset);
query = query.offset(numeric_offset);
}
let mut ids = query.load::<String>(&self.conn)?;

Expand All @@ -579,7 +580,7 @@ impl MysqlDb {

let next_offset = if limit >= 0 && ids.len() > limit as usize {
ids.pop();
Some(limit + offset)
Some((limit + numeric_offset).to_string())
} else {
None
};
Expand Down
2 changes: 1 addition & 1 deletion src/db/results.rs
View file
Expand Up @@ -56,7 +56,7 @@ where
T: Serialize,
{
pub items: Vec<T>,
pub offset: Option<i64>, // XXX: i64?
pub offset: Option<String>,
}

pub type GetBsos = Paginated<GetBso>;
Expand Down

0 comments on commit 9266f75

Please sign in to comment.