Page MenuHomePhabricator
Differential D12742

[SQLite] Add a new column to outbound_p2p_messages table
ClosedPublic
Actions

Authored by tomek on Jul 15 2024, 3:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 26 2025, 6:03 AM
Unknown Object (File)
Feb 26 2025, 6:01 AM
Unknown Object (File)
Feb 21 2025, 6:12 AM
Unknown Object (File)
Jan 26 2025, 9:58 PM
Unknown Object (File)
Jan 24 2025, 6:38 AM
Unknown Object (File)
Jan 24 2025, 6:38 AM
Unknown Object (File)
Jan 21 2025, 10:10 PM
Unknown Object (File)
Jan 16 2025, 3:55 PM
View All 60 Files
Subscribers
ashoat

Details

Reviewers
kamil
marcin
Commits
rCOMMdc46a578664e: [SQLite] Add a new column to outbound_p2p_messages table
Summary

Add a field that marks messages as being subjects to automatic retries.

https://linear.app/comm/issue/ENG-8729/automatic-retries-for-non-composable-dms

Test Plan

Open an existing app and check if the migration doesn't throw. Start a fresh app and check if the DB creation succeeds. Further testing is performed later in the stack.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Jul 15 2024, 3:07 AM
Herald added a subscriber: ashoat. · View Herald TranscriptJul 15 2024, 3:07 AM
tomek updated this revision to Diff 42279.Jul 15 2024, 3:11 AM

Format

tomek added inline comments.Jul 15 2024, 3:13 AM
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2521–2522 ↗(On Diff #42279)

These changes were made by clang-format-all

Harbormaster completed remote builds in B30317: Diff 42278.Jul 15 2024, 3:24 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 15 2024, 3:43 AM
Harbormaster failed remote builds in B30318: Diff 42279!
tomek mentioned this in D12743: Add a new field to OutboundP2PMessage.Jul 15 2024, 3:49 AM
tomek added a child revision: D12743: Add a new field to OutboundP2PMessage.Jul 15 2024, 3:50 AM
Harbormaster completed remote builds in B30318: Diff 42279.Jul 15 2024, 3:54 AM
tomek requested review of this revision.Jul 15 2024, 3:54 AM
ashoat added inline comments.Jul 15 2024, 8:13 AM
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
689 ↗(On Diff #42279)

This column name comes across as "past tense", basically indicating whether this message has been automatically retried. But from the diff description, it seems like it's more about whether the message CAN be automatically retried. Should we consider something like "can_be_automatically_retried" or "supports_automatic_retry" or "supports_auto_retry"?

812 ↗(On Diff #42279)

Nit: typo (missing space)

kamil accepted this revision.Jul 16 2024, 3:08 AM
kamil added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
689 ↗(On Diff #42279)

agree with that

This revision is now accepted and ready to land.Jul 16 2024, 3:08 AM
tomek updated this revision to Diff 42338.Jul 16 2024, 4:23 AM

Rename column

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
689 ↗(On Diff #42279)

Makes sense - renamed to supports_auto_retry

tomek updated this revision to Diff 42339.Jul 16 2024, 4:25 AM

Fix indentation

tomek marked 3 inline comments as done.Jul 16 2024, 4:28 AM
Harbormaster completed remote builds in B30375: Diff 42338.Jul 16 2024, 4:48 AM
Harbormaster completed remote builds in B30376: Diff 42339.Jul 16 2024, 4:53 AM
ashoat added a comment.Jul 16 2024, 8:30 PM

Heads-up that I landed a different migration 48 in D12759, so you'll have to rebase this one. Sorry about that!

tomek updated this revision to Diff 42386.Jul 17 2024, 8:14 AM

Rebase

Harbormaster completed remote builds in B30414: Diff 42386.Jul 17 2024, 8:31 AM
Closed by commit rCOMMdc46a578664e: [SQLite] Add a new column to outbound_p2p_messages table (authored by tomek). · Explain WhyJul 19 2024, 7:47 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMdc46a578664e: [SQLite] Add a new column to outbound_p2p_messages table.

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
DatabaseManagers/
40 lines
web/
shared-worker/
_generated/

Diff 42338

View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp

Show First 20 Lines • Show All 675 Lines • ▼ Show 20 Linesbool create_message_store_local_table(sqlite3 *db) {
std::string query = std::string query =
"CREATE TABLE IF NOT EXISTS message_store_local (" "CREATE TABLE IF NOT EXISTS message_store_local ("
" id TEXT UNIQUE PRIMARY KEY NOT NULL," " id TEXT UNIQUE PRIMARY KEY NOT NULL,"
" local_message_info TEXT NOT NULL" " local_message_info TEXT NOT NULL"
");"; ");";
return create_table(db, query, "message_store_local"); return create_table(db, query, "message_store_local");
} }
bool add_supports_auto_retry_column_to_p2p_messages_table(sqlite3 *db) {
char *error;
sqlite3_exec(
db,
"ALTER TABLE outbound_p2p_messages"
" ADD COLUMN supports_auto_retry INTEGER DEFAULT 0",
nullptr,
nullptr,
&error);
if (!error) {
return true;
}
std::ostringstream stringStream;
stringStream << "Error updating outbound_p2p_messages table: " << error;
Logger::log(stringStream.str());
sqlite3_free(error);
return false;
}
bool create_schema(sqlite3 *db) { bool create_schema(sqlite3 *db) {
char *error; char *error;
sqlite3_exec( sqlite3_exec(
db, db,
"CREATE TABLE IF NOT EXISTS drafts (" "CREATE TABLE IF NOT EXISTS drafts ("
" key TEXT UNIQUE PRIMARY KEY NOT NULL," " key TEXT UNIQUE PRIMARY KEY NOT NULL,"
" text TEXT NOT NULL" " text TEXT NOT NULL"
");" ");"
▲ Show 20 Lines • Show All 91 Lines • ▼ Show 20 Lines sqlite3_exec(
"CREATE TABLE IF NOT EXISTS outbound_p2p_messages (" "CREATE TABLE IF NOT EXISTS outbound_p2p_messages ("
" message_id TEXT NOT NULL," " message_id TEXT NOT NULL,"
" device_id TEXT NOT NULL," " device_id TEXT NOT NULL,"
" user_id TEXT NOT NULL," " user_id TEXT NOT NULL,"
" timestamp BIGINT NOT NULL," " timestamp BIGINT NOT NULL,"
" plaintext TEXT NOT NULL," " plaintext TEXT NOT NULL,"
" ciphertext TEXT NOT NULL," " ciphertext TEXT NOT NULL,"
" status TEXT NOT NULL," " status TEXT NOT NULL,"
" supports_auto_retry INTEGER DEFAULT 0,"
" PRIMARY KEY (message_id, device_id)" " PRIMARY KEY (message_id, device_id)"
");" ");"
"CREATE TABLE IF NOT EXISTS integrity_store (" "CREATE TABLE IF NOT EXISTS integrity_store ("
" id TEXT UNIQUE PRIMARY KEY NOT NULL," " id TEXT UNIQUE PRIMARY KEY NOT NULL,"
" thread_hash TEXT NOT NULL" " thread_hash TEXT NOT NULL"
");" ");"
▲ Show 20 Lines • Show All 281 Lines • ▼ Show 20 Linesstd::vector<std::pair<unsigned int, SQLiteMigration>> migrations{
{39, {create_synced_metadata_table, true}}, {39, {create_synced_metadata_table, true}},
{40, {create_keyservers_synced, true}}, {40, {create_keyservers_synced, true}},
{41, {create_aux_user_table, true}}, {41, {create_aux_user_table, true}},
{42, {add_version_column_to_olm_persist_sessions_table, true}}, {42, {add_version_column_to_olm_persist_sessions_table, true}},
{43, {create_thread_activity_table, true}}, {43, {create_thread_activity_table, true}},
{44, {create_received_messages_to_device, true}}, {44, {create_received_messages_to_device, true}},
{45, {recreate_outbound_p2p_messages_table, true}}, {45, {recreate_outbound_p2p_messages_table, true}},
{46, {create_entries_table, true}}, {46, {create_entries_table, true}},
{47, {create_message_store_local_table, true}}}}; {47, {create_message_store_local_table, true}},
{48, {add_supports_auto_retry_column_to_p2p_messages_table, true}}}};
enum class MigrationResult { SUCCESS, FAILURE, NOT_APPLIED }; enum class MigrationResult { SUCCESS, FAILURE, NOT_APPLIED };
MigrationResult applyMigrationWithTransaction( MigrationResult applyMigrationWithTransaction(
sqlite3 *db, sqlite3 *db,
const MigrateFunction &migrate, const MigrateFunction &migrate,
int index) { int index) {
sqlite3_exec(db, "BEGIN TRANSACTION;", nullptr, nullptr, nullptr); sqlite3_exec(db, "BEGIN TRANSACTION;", nullptr, nullptr, nullptr);
▲ Show 20 Lines • Show All 1,392 Lines • ▼ Show 20 Linesvoid SQLiteQueryExecutor::createMainCompaction(std::string backupID) const {
std::string tempAttachmentsPath = finalAttachmentsPath + "_tmp"; std::string tempAttachmentsPath = finalAttachmentsPath + "_tmp";
if (file_exists(tempBackupPath)) { if (file_exists(tempBackupPath)) {
Logger::log( Logger::log(
"Attempting to delete temporary backup file from previous backup " "Attempting to delete temporary backup file from previous backup "
"attempt."); "attempt.");
attempt_delete_file( attempt_delete_file(
tempBackupPath, tempBackupPath,
"Failed to delete temporary backup file from previous backup attempt."); "Failed to delete temporary backup file from previous backup "
"attempt.");
} }
if (file_exists(tempAttachmentsPath)) { if (file_exists(tempAttachmentsPath)) {
Logger::log( Logger::log(
"Attempting to delete temporary attachments file from previous backup " "Attempting to delete temporary attachments file from previous "
"backup "
"attempt."); "attempt.");
attempt_delete_file( attempt_delete_file(
tempAttachmentsPath, tempAttachmentsPath,
"Failed to delete temporary attachments file from previous backup " "Failed to delete temporary attachments file from previous backup "
"attempt."); "attempt.");
} }
sqlite3 *backupDB; sqlite3 *backupDB;
Show All 34 Linesvoid SQLiteQueryExecutor::createMainCompaction(std::string backupID) const {
} }
executeQuery(backupDB, "VACUUM;"); executeQuery(backupDB, "VACUUM;");
sqlite3_close(backupDB); sqlite3_close(backupDB);
attempt_rename_file( attempt_rename_file(
tempBackupPath, tempBackupPath,
finalBackupPath, finalBackupPath,
"Failed to rename complete temporary backup file to final backup file."); "Failed to rename complete temporary backup file to final backup "
"file.");
std::ofstream tempAttachmentsFile(tempAttachmentsPath); std::ofstream tempAttachmentsFile(tempAttachmentsPath);
if (!tempAttachmentsFile.is_open()) { if (!tempAttachmentsFile.is_open()) {
throw std::runtime_error( throw std::runtime_error(
"Unable to create attachments file for backup id: " + backupID); "Unable to create attachments file for backup id: " + backupID);
} }
std::string getAllBlobServiceMediaSQL = std::string getAllBlobServiceMediaSQL =
▲ Show 20 Lines • Show All 78 Lines • ▼ Show 20 Lines
// We don't want to run `PRAGMA key = ...;` // We don't want to run `PRAGMA key = ...;`
// on main web database. The context is here: // on main web database. The context is here:
// https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web // https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web
#ifdef EMSCRIPTEN #ifdef EMSCRIPTEN
std::string plaintextBackupPath = mainCompactionPath + "_plaintext"; std::string plaintextBackupPath = mainCompactionPath + "_plaintext";
if (file_exists(plaintextBackupPath)) { if (file_exists(plaintextBackupPath)) {
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext backup file from previous backup attempt."); "Failed to delete plaintext backup file from previous backup "
"attempt.");
} }
std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" + std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" +
mainCompactionEncryptionKey + mainCompactionEncryptionKey +
"'\";" "'\";"
"ATTACH DATABASE '" + "ATTACH DATABASE '" +
plaintextBackupPath + plaintextBackupPath +
"' AS plaintext KEY '';" "' AS plaintext KEY '';"
▲ Show 20 Lines • Show All 83 Lines • Show Last 20 Lines
View Options

web/shared-worker/_generated/comm_query_executor.wasm

Show First 20 Lines • Show All 675 Lines • ▼ Show 20 Linesbool create_message_store_local_table(sqlite3 *db) {
std::string query = std::string query =
"CREATE TABLE IF NOT EXISTS message_store_local (" "CREATE TABLE IF NOT EXISTS message_store_local ("
" id TEXT UNIQUE PRIMARY KEY NOT NULL," " id TEXT UNIQUE PRIMARY KEY NOT NULL,"
" local_message_info TEXT NOT NULL" " local_message_info TEXT NOT NULL"
");"; ");";
return create_table(db, query, "message_store_local"); return create_table(db, query, "message_store_local");
} }
bool add_supports_auto_retry_column_to_p2p_messages_table(sqlite3 *db) {
char *error;
sqlite3_exec(
db,
"ALTER TABLE outbound_p2p_messages"
" ADD COLUMN supports_auto_retry INTEGER DEFAULT 0",
nullptr,
nullptr,
&error);
if (!error) {
return true;
}
std::ostringstream stringStream;
stringStream << "Error updating outbound_p2p_messages table: " << error;
Logger::log(stringStream.str());
sqlite3_free(error);
return false;
}
bool create_schema(sqlite3 *db) { bool create_schema(sqlite3 *db) {
char *error; char *error;
sqlite3_exec( sqlite3_exec(
db, db,
"CREATE TABLE IF NOT EXISTS drafts (" "CREATE TABLE IF NOT EXISTS drafts ("
" key TEXT UNIQUE PRIMARY KEY NOT NULL," " key TEXT UNIQUE PRIMARY KEY NOT NULL,"
" text TEXT NOT NULL" " text TEXT NOT NULL"
");" ");"
▲ Show 20 Lines • Show All 91 Lines • ▼ Show 20 Lines sqlite3_exec(
"CREATE TABLE IF NOT EXISTS outbound_p2p_messages (" "CREATE TABLE IF NOT EXISTS outbound_p2p_messages ("
" message_id TEXT NOT NULL," " message_id TEXT NOT NULL,"
" device_id TEXT NOT NULL," " device_id TEXT NOT NULL,"
" user_id TEXT NOT NULL," " user_id TEXT NOT NULL,"
" timestamp BIGINT NOT NULL," " timestamp BIGINT NOT NULL,"
" plaintext TEXT NOT NULL," " plaintext TEXT NOT NULL,"
" ciphertext TEXT NOT NULL," " ciphertext TEXT NOT NULL,"
" status TEXT NOT NULL," " status TEXT NOT NULL,"
" supports_auto_retry INTEGER DEFAULT 0,"
" PRIMARY KEY (message_id, device_id)" " PRIMARY KEY (message_id, device_id)"
");" ");"
"CREATE TABLE IF NOT EXISTS integrity_store (" "CREATE TABLE IF NOT EXISTS integrity_store ("
" id TEXT UNIQUE PRIMARY KEY NOT NULL," " id TEXT UNIQUE PRIMARY KEY NOT NULL,"
" thread_hash TEXT NOT NULL" " thread_hash TEXT NOT NULL"
");" ");"
▲ Show 20 Lines • Show All 281 Lines • ▼ Show 20 Linesstd::vector<std::pair<unsigned int, SQLiteMigration>> migrations{
{39, {create_synced_metadata_table, true}}, {39, {create_synced_metadata_table, true}},
{40, {create_keyservers_synced, true}}, {40, {create_keyservers_synced, true}},
{41, {create_aux_user_table, true}}, {41, {create_aux_user_table, true}},
{42, {add_version_column_to_olm_persist_sessions_table, true}}, {42, {add_version_column_to_olm_persist_sessions_table, true}},
{43, {create_thread_activity_table, true}}, {43, {create_thread_activity_table, true}},
{44, {create_received_messages_to_device, true}}, {44, {create_received_messages_to_device, true}},
{45, {recreate_outbound_p2p_messages_table, true}}, {45, {recreate_outbound_p2p_messages_table, true}},
{46, {create_entries_table, true}}, {46, {create_entries_table, true}},
{47, {create_message_store_local_table, true}}}}; {47, {create_message_store_local_table, true}},
{48, {add_supports_auto_retry_column_to_p2p_messages_table, true}}}};
enum class MigrationResult { SUCCESS, FAILURE, NOT_APPLIED }; enum class MigrationResult { SUCCESS, FAILURE, NOT_APPLIED };
MigrationResult applyMigrationWithTransaction( MigrationResult applyMigrationWithTransaction(
sqlite3 *db, sqlite3 *db,
const MigrateFunction &migrate, const MigrateFunction &migrate,
int index) { int index) {
sqlite3_exec(db, "BEGIN TRANSACTION;", nullptr, nullptr, nullptr); sqlite3_exec(db, "BEGIN TRANSACTION;", nullptr, nullptr, nullptr);
▲ Show 20 Lines • Show All 1,392 Lines • ▼ Show 20 Linesvoid SQLiteQueryExecutor::createMainCompaction(std::string backupID) const {
std::string tempAttachmentsPath = finalAttachmentsPath + "_tmp"; std::string tempAttachmentsPath = finalAttachmentsPath + "_tmp";
if (file_exists(tempBackupPath)) { if (file_exists(tempBackupPath)) {
Logger::log( Logger::log(
"Attempting to delete temporary backup file from previous backup " "Attempting to delete temporary backup file from previous backup "
"attempt."); "attempt.");
attempt_delete_file( attempt_delete_file(
tempBackupPath, tempBackupPath,
"Failed to delete temporary backup file from previous backup attempt."); "Failed to delete temporary backup file from previous backup "
"attempt.");
} }
if (file_exists(tempAttachmentsPath)) { if (file_exists(tempAttachmentsPath)) {
Logger::log( Logger::log(
"Attempting to delete temporary attachments file from previous backup " "Attempting to delete temporary attachments file from previous "
"backup "
"attempt."); "attempt.");
attempt_delete_file( attempt_delete_file(
tempAttachmentsPath, tempAttachmentsPath,
"Failed to delete temporary attachments file from previous backup " "Failed to delete temporary attachments file from previous backup "
"attempt."); "attempt.");
} }
sqlite3 *backupDB; sqlite3 *backupDB;
Show All 34 Linesvoid SQLiteQueryExecutor::createMainCompaction(std::string backupID) const {
} }
executeQuery(backupDB, "VACUUM;"); executeQuery(backupDB, "VACUUM;");
sqlite3_close(backupDB); sqlite3_close(backupDB);
attempt_rename_file( attempt_rename_file(
tempBackupPath, tempBackupPath,
finalBackupPath, finalBackupPath,
"Failed to rename complete temporary backup file to final backup file."); "Failed to rename complete temporary backup file to final backup "
"file.");
std::ofstream tempAttachmentsFile(tempAttachmentsPath); std::ofstream tempAttachmentsFile(tempAttachmentsPath);
if (!tempAttachmentsFile.is_open()) { if (!tempAttachmentsFile.is_open()) {
throw std::runtime_error( throw std::runtime_error(
"Unable to create attachments file for backup id: " + backupID); "Unable to create attachments file for backup id: " + backupID);
} }
std::string getAllBlobServiceMediaSQL = std::string getAllBlobServiceMediaSQL =
▲ Show 20 Lines • Show All 78 Lines • ▼ Show 20 Lines
// We don't want to run `PRAGMA key = ...;` // We don't want to run `PRAGMA key = ...;`
// on main web database. The context is here: // on main web database. The context is here:
// https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web // https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web
#ifdef EMSCRIPTEN #ifdef EMSCRIPTEN
std::string plaintextBackupPath = mainCompactionPath + "_plaintext"; std::string plaintextBackupPath = mainCompactionPath + "_plaintext";
if (file_exists(plaintextBackupPath)) { if (file_exists(plaintextBackupPath)) {
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext backup file from previous backup attempt."); "Failed to delete plaintext backup file from previous backup "
"attempt.");
} }
std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" + std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" +
mainCompactionEncryptionKey + mainCompactionEncryptionKey +
"'\";" "'\";"
"ATTACH DATABASE '" + "ATTACH DATABASE '" +
plaintextBackupPath + plaintextBackupPath +
"' AS plaintext KEY '';" "' AS plaintext KEY '';"
▲ Show 20 Lines • Show All 83 Lines • Show Last 20 Lines