FreshRSS/rssguard
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix some DB race conditions, fix some bulk insert SQL formatting bugs along the way

Browse Source
This commit is contained in:
Martin Rotter 2023-01-11 12:31:07 +01:00
parent 714d1998ba
commit 1f988b8a60
8 changed files with 37 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/librssguard/core/feeddownloader.cpp
View File

@ -317,15 +317,15 @@ void FeedDownloader::updateOneFeed(ServiceRoot* acc,
}
if (!msg_original.m_isRead && msg_tweaked_by_filter->m_isRead) {
qDebugNN << LOGSEC_FEEDDOWNLOADER << "Message with custom ID: '" << msg_original.m_customId
<< "' was marked as read by message scripts.";
qDebugNN << LOGSEC_FEEDDOWNLOADER << "Message with custom ID:" << QUOTE_W_SPACE(msg_original.m_customId)
<< "was marked as read by message scripts.";
read_msgs << *msg_tweaked_by_filter;
}
if (!msg_original.m_isImportant && msg_tweaked_by_filter->m_isImportant) {
qDebugNN << LOGSEC_FEEDDOWNLOADER << "Message with custom ID: '" << msg_original.m_customId
<< "' was marked as important by message scripts.";
qDebugNN << LOGSEC_FEEDDOWNLOADER << "Message with custom ID:" << QUOTE_W_SPACE(msg_original.m_customId)
<< "was marked as important by message scripts.";
important_msgs << *msg_tweaked_by_filter;
}
@ -333,6 +333,8 @@ void FeedDownloader::updateOneFeed(ServiceRoot* acc,
// Process changed labels.
for (Label* lbl : qAsConst(msg_original.m_assignedLabels)) {
if (!msg_tweaked_by_filter->m_assignedLabels.contains(lbl)) {
QMutexLocker lck(&m_mutexDb);
// Label is not there anymore, it was deassigned.
lbl->deassignFromMessage(*msg_tweaked_by_filter);
@ -344,6 +346,8 @@ void FeedDownloader::updateOneFeed(ServiceRoot* acc,
for (Label* lbl : qAsConst(msg_tweaked_by_filter->m_assignedLabels)) {
if (!msg_original.m_assignedLabels.contains(lbl)) {
QMutexLocker lck(&m_mutexDb);
// Label is in new message, but is not in old message, it
// was newly assigned.
lbl->assignToMessage(*msg_tweaked_by_filter);
@ -393,7 +397,7 @@ void FeedDownloader::updateOneFeed(ServiceRoot* acc,
removeDuplicateMessages(msgs);
tmr.restart();
auto updated_messages = acc->updateMessages(msgs, feed, false);
auto updated_messages = acc->updateMessages(msgs, feed, false, &m_mutexDb);
qDebugNN << LOGSEC_FEEDDOWNLOADER << "Updating messages in DB took" << NONQUOTE_W_SPACE(tmr.nsecsElapsed() / 1000)
<< "microseconds.";

1
src/librssguard/core/feeddownloader.h
View File

@ -78,6 +78,7 @@ class FeedDownloader : public QObject {
private:
bool m_isCacheSynchronizationRunning;
bool m_stopCacheSynchronization;
QMutex m_mutexDb;
QHash<ServiceRoot*, ApplicationException> m_erroredAccounts;
QList<FeedUpdateRequest> m_feeds = {};
QFutureWatcher<FeedUpdateResult> m_watcherLookup;

32
src/librssguard/database/databasequeries.cpp
View File

@ -1080,6 +1080,7 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
QList<Message>& messages,
Feed* feed,
bool force_update,
QMutex* db_mutex,
bool* ok) {
if (messages.isEmpty()) {
*ok = true;
@ -1096,7 +1097,6 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
QSqlQuery query_select_with_custom_id_for_feed(db);
QSqlQuery query_select_with_id(db);
QSqlQuery query_update(db);
QSqlQuery query_insert(db);
// Here we have query which will check for existence of the "same" message in given feed.
// The two message are the "same" if:
@ -1130,14 +1130,6 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
.prepare(QSL("SELECT date_created, is_read, is_important, contents, feed, title, author FROM Messages "
"WHERE id = :id AND account_id = :account_id;"));
// Used to insert new messages.
query_insert.setForwardOnly(true);
query_insert.prepare(QSL("INSERT INTO Messages "
"(feed, title, is_read, is_important, is_deleted, url, author, score, date_created, "
"contents, enclosures, custom_id, custom_hash, account_id) "
"VALUES (:feed, :title, :is_read, :is_important, :is_deleted, :url, :author, :score, "
":date_created, :contents, :enclosures, :custom_id, :custom_hash, :account_id);"));
// Used to update existing messages.
query_update.setForwardOnly(true);
query_update.prepare(QSL("UPDATE Messages "
@ -1321,6 +1313,8 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
(!ignore_contents_changes && message.m_contents != contents_existing_message);
if (cond_1 || cond_2 || cond_3 || force_update) {
QMutexLocker lck(db_mutex);
// Message exists and is changed, update it.
query_update.bindValue(QSL(":title"), unnulifyString(message.m_title));
query_update.bindValue(QSL(":is_read"), int(message.m_isRead));
@ -1349,8 +1343,8 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
updated_messages.second++;
}
else if (query_update.lastError().isValid()) {
qWarningNN << LOGSEC_DB
<< "Failed to update message in DB:" << QUOTE_W_SPACE_DOT(query_update.lastError().text());
qCriticalNN << LOGSEC_DB
<< "Failed to update message in DB:" << QUOTE_W_SPACE_DOT(query_update.lastError().text());
}
query_update.finish();
@ -1407,7 +1401,7 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
.replace(QSL(":date_created"), QString::number(msg->m_created.toMSecsSinceEpoch()))
.replace(QSL(":contents"), DatabaseFactory::escapeQuery(unnulifyString(msg->m_contents)))
.replace(QSL(":enclosures"), Enclosures::encodeEnclosuresToString(msg->m_enclosures))
.replace(QSL(":custom_id"), unnulifyString(msg->m_customId))
.replace(QSL(":custom_id"), DatabaseFactory::escapeQuery(unnulifyString(msg->m_customId)))
.replace(QSL(":custom_hash"), unnulifyString(msg->m_customHash))
.replace(QSL(":score"), QString::number(msg->m_score))
.replace(QSL(":account_id"), QString::number(account_id)));
@ -1415,6 +1409,9 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
if (!vals.isEmpty()) {
QString final_bulk = bulk_insert.arg(vals.join(QSL(", ")));
QMutexLocker lck(db_mutex);
auto bulk_query = db.exec(final_bulk);
auto bulk_error = bulk_query.lastError();
@ -1422,6 +1419,8 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
QString txt = bulk_error.text() + bulk_error.databaseText() + bulk_error.driverText();
qCriticalNN << LOGSEC_DB << "Failed bulk insert of articles:" << QUOTE_W_SPACE_DOT(txt);
IOFactory::writeFile("aaa.sql", final_bulk.toUtf8());
}
else {
// OK, we bulk-inserted many messages but the thing is that they do not
@ -1446,23 +1445,26 @@ QPair<int, int> DatabaseQueries::updateMessages(QSqlDatabase db,
for (Message& message : messages) {
if (!message.m_assignedLabels.isEmpty()) {
if (!message.m_customId.isEmpty() || message.m_id > 0) {
QMutexLocker lck(db_mutex);
setLabelsForMessage(db, message.m_assignedLabels, message);
}
else {
qWarningNN << LOGSEC_DB << "Cannot set labels for message" << QUOTE_W_SPACE(message.m_title)
<< "because we don't have ID or custom ID.";
qCriticalNN << LOGSEC_DB << "Cannot set labels for message" << QUOTE_W_SPACE(message.m_title)
<< "because we don't have ID or custom ID.";
}
}
}
// Now, fixup custom IDS for messages which initially did not have them,
// just to keep the data consistent.
QMutexLocker lck(db_mutex);
if (db.exec("UPDATE Messages "
"SET custom_id = id "
"WHERE custom_id IS NULL OR custom_id = '';")
.lastError()
.isValid()) {
qWarningNN << LOGSEC_DB << "Failed to set custom ID for all messages:" << QUOTE_W_SPACE_DOT(db.lastError().text());
qCriticalNN << LOGSEC_DB << "Failed to set custom ID for all messages:" << QUOTE_W_SPACE_DOT(db.lastError().text());
}
if (ok != nullptr) {

1
src/librssguard/database/databasequeries.h
View File

@ -140,6 +140,7 @@ class DatabaseQueries {
QList<Message>& messages,
Feed* feed,
bool force_update,
QMutex* db_mutex,
bool* ok = nullptr);
static bool deleteAccount(const QSqlDatabase& db, ServiceRoot* account);
static bool deleteAccountData(const QSqlDatabase& db,

2
src/librssguard/gui/dialogs/formmessagefiltersmanager.cpp
View File

@ -448,7 +448,7 @@ void FormMessageFiltersManager::processCheckedFeeds() {
}
// Update messages in DB and reload selection.
it->getParentServiceRoot()->updateMessages(msgs, it->toFeed(), true);
it->getParentServiceRoot()->updateMessages(msgs, it->toFeed(), true, nullptr);
displayMessagesOfFeed();
}
}

6
src/librssguard/services/abstract/serviceroot.cpp
View File

@ -943,7 +943,7 @@ ServiceRoot::LabelOperation operator&(ServiceRoot::LabelOperation lhs, ServiceRo
return static_cast<ServiceRoot::LabelOperation>(static_cast<char>(lhs) & static_cast<char>(rhs));
}
QPair<int, int> ServiceRoot::updateMessages(QList<Message>& messages, Feed* feed, bool force_update) {
QPair<int, int> ServiceRoot::updateMessages(QList<Message>& messages, Feed* feed, bool force_update, QMutex* db_mutex) {
QPair<int, int> updated_messages = {0, 0};
if (messages.isEmpty()) {
@ -956,9 +956,11 @@ QPair<int, int> ServiceRoot::updateMessages(QList<Message>& messages, Feed* feed
qDebugNN << LOGSEC_CORE << "Updating messages in DB.";
updated_messages = DatabaseQueries::updateMessages(database, messages, feed, force_update, &ok);
updated_messages = DatabaseQueries::updateMessages(database, messages, feed, force_update, db_mutex, &ok);
if (updated_messages.first > 0 || updated_messages.second > 0) {
QMutexLocker lck(db_mutex);
// Something was added or updated in the DB, update numbers.
feed->updateCounts(true);

3
src/librssguard/services/abstract/serviceroot.h
View File

@ -14,6 +14,7 @@
#include <QPair>
class QAction;
class QMutex;
class FeedsModel;
class RecycleBin;
class ImportantNode;
@ -197,7 +198,7 @@ class ServiceRoot : public RootItem {
void completelyRemoveAllData();
// Returns counts of updated messages <unread, all>.
QPair<int, int> updateMessages(QList<Message>& messages, Feed* feed, bool force_update);
QPair<int, int> updateMessages(QList<Message>& messages, Feed* feed, bool force_update, QMutex* db_mutex);
QIcon feedIconForMessage(const QString& feed_custom_id) const;

3
src/librssguard/services/standard/gui/formstandardimportexport.cpp
View File

@ -248,7 +248,8 @@ void FormStandardImportExport::parseImportFile(const QString& file_name, bool fe
QFile input_file(file_name);
QByteArray input_data;
if (input_file.open(QIODevice::Text | QIODevice::Unbuffered | QIODevice::ReadOnly)) {
if (input_file.open(QIODevice::OpenModeFlag::Text | QIODevice::OpenModeFlag::Unbuffered |
QIODevice::OpenModeFlag::ReadOnly)) {
input_data = input_file.readAll();
input_file.close();
}