Skip to content

Commit

Permalink
Avoid double reverse
Browse files Browse the repository at this point in the history
  • Loading branch information
iambriccardo committed Sep 11, 2024
1 parent 6650a46 commit b280d31
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 17 deletions.
7 changes: 1 addition & 6 deletions relay-server/src/services/buffer/envelope_stack/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl SqliteEnvelopeStack {
/// In case an envelope fails deserialization due to malformed data in the database, the affected
/// envelope will not be unspooled and unspooling will continue with the remaining envelopes.
async fn unspool_from_disk(&mut self) -> Result<(), SqliteEnvelopeStackError> {
let mut envelopes = relay_statsd::metric!(timer(RelayTimers::BufferUnspool), {
let envelopes = relay_statsd::metric!(timer(RelayTimers::BufferUnspool), {
self.envelope_store
.delete_many(
self.own_key,
Expand All @@ -139,11 +139,6 @@ impl SqliteEnvelopeStack {
return Ok(());
}

// Since the store returns the envelopes sorted in descending order, we want to put them
// in reverse into the vector in the buffer, because we want to pop the last element always,
// which has to be the newest (aka with the biggest timestamp).
envelopes.reverse();

// We push in the back of the buffer, since we still want to give priority to
// incoming envelopes that have a more recent timestamp.
self.batches_buffer_size += envelopes.len();
Expand Down
15 changes: 4 additions & 11 deletions relay-server/src/services/buffer/envelope_store/sqlite.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::cmp::Reverse;
use std::error::Error;
use std::path::Path;
use std::pin::pin;
Expand Down Expand Up @@ -371,12 +370,12 @@ impl SqliteEnvelopeStore {
}
}

// We sort envelopes by `received_at`.
// We sort envelopes by `received_at` in ascending order.
//
// Unfortunately we have to do this because SQLite `DELETE` with `RETURNING` doesn't
// return deleted rows in a specific order.
extracted_envelopes.sort_by_key(|a| {
Reverse(UnixTimestamp::from_datetime(a.received_at()).unwrap_or(UnixTimestamp::now()))
UnixTimestamp::from_datetime(a.received_at()).unwrap_or(UnixTimestamp::now())
});

Ok(extracted_envelopes)
Expand Down Expand Up @@ -553,10 +552,7 @@ mod tests {
.unwrap();
assert_eq!(extracted_envelopes.len(), 5);
for (i, extracted_envelope) in extracted_envelopes.iter().enumerate().take(5) {
assert_eq!(
extracted_envelope.event_id(),
envelopes[5..][4 - i].event_id()
);
assert_eq!(extracted_envelope.event_id(), envelopes[5..][i].event_id());
}

// We check that if we load more than the envelopes stored on disk, we still get back at
Expand All @@ -567,10 +563,7 @@ mod tests {
.unwrap();
assert_eq!(extracted_envelopes.len(), 5);
for (i, extracted_envelope) in extracted_envelopes.iter().enumerate().take(5) {
assert_eq!(
extracted_envelope.event_id(),
envelopes[0..5][4 - i].event_id()
);
assert_eq!(extracted_envelope.event_id(), envelopes[0..5][i].event_id());
}
}

Expand Down

0 comments on commit b280d31

Please sign in to comment.