Add deduping to mobile packet verifier #585
Conversation
| @@ -0,0 +1,4 @@ | |||
| CREATE TABLE event_ids ( | |||
| event_id TEXT NOT NULL PRIMARY KEY, | |||
There was a problem hiding this comment.
what are these event_ids constituted from? are we pretty sure they'll be unique for the life of a record in this database without aggregating to some other piece of data like a hotspot pubkey or cbsd_id?
There was a problem hiding this comment.
There are unique according to Dmytro.
| @@ -80,17 +70,29 @@ pub async fn accumulate_sessions( | |||
| } | |||
|
|
|||
| async fn verify_report( | |||
| tx: &mut Transaction<'_, Postgres>, | |||
There was a problem hiding this comment.
| tx: &mut Transaction<'_, Postgres>, | |
| txn: &mut Transaction<'_, Postgres>, |
tx reads like a message channel sender
| pub async fn is_duplicate( | ||
| tx: &mut Transaction<'_, Postgres>, | ||
| event_id: String, | ||
| ) -> anyhow::Result<bool> { | ||
| let result = | ||
| sqlx::query_scalar::<_, bool>("SELECT EXISTS(SELECT 1 FROM event_ids WHERE event_id = $1)") | ||
| .bind(event_id.clone()) | ||
| .fetch_one(&mut *tx) | ||
| .await?; | ||
|
|
||
| Ok(result) | ||
| } | ||
|
|
||
| pub async fn record( | ||
| tx: &mut Transaction<'_, Postgres>, | ||
| event_id: String, | ||
| received_timestamp: DateTime<Utc>, | ||
| ) -> anyhow::Result<()> { | ||
| sqlx::query("INSERT INTO event_ids(event_id, received_timestamp) VALUES($1,$2)") | ||
| .bind(event_id) | ||
| .bind(received_timestamp) | ||
| .execute(tx) | ||
| .await | ||
| .map(|_| ()) | ||
| .map_err(anyhow::Error::from) | ||
| } |
There was a problem hiding this comment.
worth the speed boost to make this an upsert statement returning whether or not the insert happened and then do it in a single query?
There was a problem hiding this comment.
i've done this in a different app which attempts to do an upsert and then returns the boolean if the row was inserted or not
query_with_bindings.execute(&mut tx).await?.rows_affected() > 0;
There was a problem hiding this comment.
I agree with @jeffgrunewald here, this should be an upsert
There was a problem hiding this comment.
This is would the query would look like:
let is_duplicate = sqlx::query(
r#"
INSERT INTO event_ids (event_id, received_timestamp) VALUES ($1, $2)
ON CONFLICT DO NOTHING
"#
)
.bind(event_id)
.bind(received_timestamp)
.execute(tx)
.await?
.rows_affected() > 0;There was a problem hiding this comment.
Agreed, upsert is better 5c66469
| event_id: String, | ||
| received_timestamp: DateTime<Utc>, | ||
| ) -> anyhow::Result<()> { | ||
| sqlx::query("INSERT INTO event_ids(event_id, received_timestamp) VALUES($1,$2)") |
There was a problem hiding this comment.
| sqlx::query("INSERT INTO event_ids(event_id, received_timestamp) VALUES($1,$2)") | |
| sqlx::query("INSERT INTO event_ids(event_id, received_timestamp) VALUES ($1, $2)") |
No description provided.