Skip to content

Commit 8bd7ba4

Browse files
committed
Bug 1897264 - interruption support for ingestion
Added an `interrupt_everything` method that interrupts both the read and write connection. This is intended for iOS to use to interrupt the suggest component during shutdown. See https://bugzilla.mozilla.org/show_bug.cgi?id=1897299 for a discussion on next steps here. Created a new `WriteScope` type that stores an interrupt scope that can be used for multiple `write` calls. This allows the ingestion code to catch all interruptions that happened after the ingestion started. With the old system, if `interrupt_everything` was called in-between ingestion two types then we might miss it.
1 parent 1ebbd1a commit 8bd7ba4

File tree

5 files changed

+132
-68
lines changed

5 files changed

+132
-68
lines changed

components/suggest/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ While the store provides most of the functionality, the application has a few re
2020

2121
**1. Create and manage a `SuggestStore` as a singleton.** Under the hood, the store holds multiple connections to the database: a read-write connection for ingestion, and a read-only connection for queries. The store uses the right connection for each operation, so applications shouldn't create multiple stores. The application is responsible for specifying the correct platform-specific storage directory for the database. The database contains _cached data_, like suggestions, and _user data_, like which suggestions have been dismissed. For this reason, applications should persist the database in their durable storage or "profile" directory. Applications specify the storage directory, and create a store, using the `SuggestStoreBuilder` interface.
2222

23-
**2. Periodically call the store's `ingest()` method to ingest new suggestions.** While the store takes care of efficiently downloading and persisting suggestions from Remote Settings, the application is still responsible for scheduling this work. This is because the Suggest component doesn't have access to the various platform-specific background work scheduling mechanisms, like `nsIUpdateTimerManager` on Desktop, `WorkManager` on Android, or `BGTaskScheduler` on iOS. These are three different APIs with different constraints, written in three different languages. Instead of trying to bind to these different mechanisms, the component leaves it up to the application to use the right one on each platform. Ingestion is network- and disk I/O-bound, and should be done in the background.
23+
**2. Periodically call the store's `ingest()` method to ingest new suggestions.** While the store takes care of efficiently downloading and persisting suggestions from Remote Settings, the application is still responsible for scheduling this work. This is because the Suggest component doesn't have access to the various platform-specific background work scheduling mechanisms, like `nsIUpdateTimerManager` on Desktop, `WorkManager` on Android, or `BGTaskScheduler` on iOS. These are three different APIs with different constraints, written in three different languages. Instead of trying to bind to these different mechanisms, the component leaves it up to the application to use the right one on each platform. Ingestion is network- and disk I/O-bound, and should be done in the background. If the ingestion needs to be cancelled, call `interrupt(InterruptKind::Write)`.
2424

25-
**3. Use the store's `query()` and `interrupt()` methods to query for fresh suggestions as the user types.** The application passes the user's input, and additional options like which suggestion types to return, to `query()`. Querying the database is disk I/O-bound, so applications should use their respective platforms' facilities for asynchronous work—Kotlin coroutines on Android, and Swift actors on iOS; the UniFFI bindings for Desktop take care of dispatching work to a background thread pool—to avoid blocking the main thread. Running `query()` off-main-thread also lets applications `interrupt()` those queries from the main thread when the user's input changes. This avoids waiting for a query to return suggestions that are now stale.
25+
**3. Use the store's `query()` and `interrupt()` methods to query for fresh suggestions as the user types.** The application passes the user's input, and additional options like which suggestion types to return, to `query()`. Querying the database is disk I/O-bound, so applications should use their respective platforms' facilities for asynchronous work—Kotlin coroutines on Android, and Swift actors on iOS; the UniFFI bindings for Desktop take care of dispatching work to a background thread pool—to avoid blocking the main thread. Running `query()` off-main-thread also lets applications `interrupt(InterruptKind::Read)` those queries from the main thread when the user's input changes. This avoids waiting for a query to return suggestions that are now stale.
2626

2727
### For contributors
2828

components/suggest/src/db.rs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use std::{collections::HashSet, path::Path, sync::Arc};
77

88
use interrupt_support::{SqlInterruptHandle, SqlInterruptScope};
9-
use parking_lot::Mutex;
9+
use parking_lot::{Mutex, MutexGuard};
1010
use rusqlite::{
1111
named_params,
1212
types::{FromSql, ToSql},
@@ -98,7 +98,7 @@ impl SuggestDb {
9898
pub fn read<T>(&self, op: impl FnOnce(&SuggestDao) -> Result<T>) -> Result<T> {
9999
let conn = self.conn.lock();
100100
let scope = self.interrupt_handle.begin_interrupt_scope()?;
101-
let dao = SuggestDao::new(&conn, scope);
101+
let dao = SuggestDao::new(&conn, &scope);
102102
op(&dao)
103103
}
104104

@@ -107,11 +107,45 @@ impl SuggestDb {
107107
let mut conn = self.conn.lock();
108108
let scope = self.interrupt_handle.begin_interrupt_scope()?;
109109
let tx = conn.transaction()?;
110-
let mut dao = SuggestDao::new(&tx, scope);
110+
let mut dao = SuggestDao::new(&tx, &scope);
111111
let result = op(&mut dao)?;
112112
tx.commit()?;
113113
Ok(result)
114114
}
115+
116+
/// Create a new write scope.
117+
///
118+
/// This enables performing multiple `write()` calls with the same shared interrupt scope.
119+
/// This is important for things like ingestion, where you want the operation to be interrupted
120+
/// if [Self::interrupt_handle::interrupt] is called after the operation starts. Calling
121+
/// [Self::write] multiple times during the operation risks missing a call that happens after
122+
/// between those calls.
123+
pub fn write_scope(&self) -> Result<WriteScope> {
124+
Ok(WriteScope {
125+
conn: self.conn.lock(),
126+
scope: self.interrupt_handle.begin_interrupt_scope()?,
127+
})
128+
}
129+
}
130+
131+
pub(crate) struct WriteScope<'a> {
132+
pub conn: MutexGuard<'a, Connection>,
133+
pub scope: SqlInterruptScope,
134+
}
135+
136+
impl WriteScope<'_> {
137+
/// Accesses the Suggest database in a transaction for reading and writing.
138+
pub fn write<T>(&mut self, op: impl FnOnce(&mut SuggestDao) -> Result<T>) -> Result<T> {
139+
let tx = self.conn.transaction()?;
140+
let mut dao = SuggestDao::new(&tx, &self.scope);
141+
let result = op(&mut dao)?;
142+
tx.commit()?;
143+
Ok(result)
144+
}
145+
146+
pub fn err_if_interrupted(&self) -> Result<()> {
147+
Ok(self.scope.err_if_interrupted()?)
148+
}
115149
}
116150

117151
/// A data access object (DAO) that wraps a connection to the Suggest database
@@ -122,11 +156,11 @@ impl SuggestDb {
122156
/// reference (`&mut self`).
123157
pub(crate) struct SuggestDao<'a> {
124158
pub conn: &'a Connection,
125-
pub scope: SqlInterruptScope,
159+
pub scope: &'a SqlInterruptScope,
126160
}
127161

128162
impl<'a> SuggestDao<'a> {
129-
fn new(conn: &'a Connection, scope: SqlInterruptScope) -> Self {
163+
fn new(conn: &'a Connection, scope: &'a SqlInterruptScope) -> Self {
130164
Self { conn, scope }
131165
}
132166

components/suggest/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub use config::{SuggestGlobalConfig, SuggestProviderConfig};
2525
pub use error::SuggestApiError;
2626
pub use provider::SuggestionProvider;
2727
pub use query::SuggestionQuery;
28-
pub use store::{SuggestIngestionConstraints, SuggestStore, SuggestStoreBuilder};
28+
pub use store::{InterruptKind, SuggestIngestionConstraints, SuggestStore, SuggestStoreBuilder};
2929
pub use suggestion::{raw_suggestion_url_matches, Suggestion};
3030

3131
pub(crate) type Result<T> = std::result::Result<T, error::Error>;

0 commit comments

Comments
 (0)