Branch 'kolab/integration/4.13.0' - 3 commits - akonadi/monitor_p.cpp akonadi/monitor_p.h
Christian Mollekopf
mollekopf at kolabsys.com
Thu Oct 16 17:33:58 CEST 2014
akonadi/monitor_p.cpp | 49 ++++++++++++++++++++++++++++---------------------
akonadi/monitor_p.h | 2 ++
2 files changed, 30 insertions(+), 21 deletions(-)
New commits:
commit 55f2937231ec2b9da57696475692b0389a8f292e
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Wed Oct 15 21:34:15 2014 +0200
Monitor: Correctly deal with invalid notification (ignore them).
* If we don't fetch items or collections we still have to emit the signal.
* Only clean notification if we tried to emit a signal, but noone listened (otherwise we run into this inefficient codepath a lot)
diff --git a/akonadi/monitor_p.cpp b/akonadi/monitor_p.cpp
index 6b44efb..217d29f 100644
--- a/akonadi/monitor_p.cpp
+++ b/akonadi/monitor_p.cpp
@@ -420,6 +420,16 @@ void MonitorPrivate::cleanOldNotifications()
}
}
+bool MonitorPrivate::fetchCollections() const
+{
+ return fetchCollection;
+}
+
+bool MonitorPrivate::fetchItems() const
+{
+ return !mItemFetchScope.isEmpty();
+}
+
bool MonitorPrivate::ensureDataAvailable(const NotificationMessageV3 &msg)
{
if (msg.type() == NotificationMessageV2::Tags) {
@@ -441,7 +451,7 @@ bool MonitorPrivate::ensureDataAvailable(const NotificationMessageV3 &msg)
}
bool allCached = true;
- if (fetchCollection) {
+ if (fetchCollections()) {
if (!collectionCache->ensureCached(msg.parentCollection(), mCollectionFetchScope)) {
allCached = false;
}
@@ -453,7 +463,7 @@ bool MonitorPrivate::ensureDataAvailable(const NotificationMessageV3 &msg)
return allCached; // the actual object is gone already, nothing to fetch there
}
- if (msg.type() == NotificationMessageV2::Items && !mItemFetchScope.isEmpty()) {
+ if (msg.type() == NotificationMessageV2::Items && fetchItems()) {
ItemFetchScope scope(mItemFetchScope);
if (mFetchChangedOnly && (msg.operation() == NotificationMessageV2::Modify || msg.operation() == NotificationMessageV2::ModifyFlags)) {
bool fullPayloadWasRequested = scope.fullPayload();
@@ -493,7 +503,7 @@ bool MonitorPrivate::ensureDataAvailable(const NotificationMessageV3 &msg)
}
}
- } else if (msg.type() == NotificationMessageV2::Collections && fetchCollection) {
+ } else if (msg.type() == NotificationMessageV2::Collections && fetchCollections()) {
Q_FOREACH (const NotificationMessageV2::Entity &entity, msg.entities()) {
if (!collectionCache->ensureCached(entity.id, mCollectionFetchScope)) {
allCached = false;
@@ -507,10 +517,12 @@ bool MonitorPrivate::ensureDataAvailable(const NotificationMessageV3 &msg)
bool MonitorPrivate::emitNotification(const NotificationMessageV3 &msg)
{
bool someoneWasListening = false;
+ bool shouldCleanOldNotifications = false;
if (msg.type() == NotificationMessageV2::Tags) {
//In case of a Remove notification this will return a list of invalid entities (we'll deal later with them)
const Tag::List tags = tagCache->retrieve(msg.uids());
someoneWasListening = emitTagsNotification(msg, tags);
+ shouldCleanOldNotifications = !someoneWasListening;
} else if (msg.type() == NotificationMessageV2::Relations) {
Relation rel;
Q_FOREACH (const QByteArray & part, msg.itemParts()) {
@@ -527,6 +539,7 @@ bool MonitorPrivate::emitNotification(const NotificationMessageV3 &msg)
}
}
someoneWasListening = emitRelationsNotification(msg, Relation::List() << rel);
+ shouldCleanOldNotifications = !someoneWasListening;
} else {
const Collection parent = collectionCache->retrieve(msg.parentCollection());
Collection destParent;
@@ -537,18 +550,28 @@ bool MonitorPrivate::emitNotification(const NotificationMessageV3 &msg)
if (msg.type() == NotificationMessageV2::Collections) {
Collection col;
Q_FOREACH (const NotificationMessageV2::Entity &entity, msg.entities()) {
+ //For removals this will retrieve an invalid collection. We'll deal with that in emitCollectionNotification
col = collectionCache->retrieve(entity.id);
- if (emitCollectionNotification(msg, col, parent, destParent) && !someoneWasListening) {
- someoneWasListening = true;
+ //It is possible that the retrieval fails also in the non-removal case (e.g. because the item was meanwhile removed while
+ //the changerecorder stored the notification or the notification was in the queue). In order to drop such invalid notifications we have to ignore them.
+ if (col.isValid() || msg.operation() == NotificationMessageV2::Remove || !fetchCollections()) {
+ someoneWasListening = emitCollectionNotification(msg, col, parent, destParent);
+ shouldCleanOldNotifications = !someoneWasListening;
}
}
} else if (msg.type() == NotificationMessageV2::Items) {
+ //For removals this will retrieve an empty set. We'll deal with that in emitItemNotification
const Item::List items = itemCache->retrieve(msg.uids());
- someoneWasListening = emitItemsNotification(msg, items, parent, destParent);
+ //It is possible that the retrieval fails also in the non-removal case (e.g. because the item was meanwhile removed while
+ //the changerecorder stored the notification or the notification was in the queue). In order to drop such invalid notifications we have to ignore them.
+ if (!items.isEmpty() || msg.operation() == NotificationMessageV2::Remove || !fetchItems()) {
+ someoneWasListening = emitItemsNotification(msg, items, parent, destParent);
+ shouldCleanOldNotifications = !someoneWasListening;
+ }
}
}
- if (!someoneWasListening) {
+ if (shouldCleanOldNotifications) {
cleanOldNotifications(); // probably someone disconnected a signal in the meantime, get rid of the no longer interesting stuff
}
diff --git a/akonadi/monitor_p.h b/akonadi/monitor_p.h
index 04c9076..ed3a0a6 100644
--- a/akonadi/monitor_p.h
+++ b/akonadi/monitor_p.h
@@ -309,6 +309,8 @@ private:
}
void notifyCollectionStatisticsWatchers(Collection::Id collection, const QByteArray &resource);
+ bool fetchCollections() const;
+ bool fetchItems() const;
};
}
commit f9251cd1aa4dabed4d540a73e7f3c1fdcfd89e7b
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Wed Oct 15 21:16:01 2014 +0200
Revert "Monitor: handle correctly situation when item no longer exists on server"
This reverts commit 1a576d44171f26d7f14e03bb80398f8534eefcd9.
Empty item list or invalid collection is a valid situation during
removals.
diff --git a/akonadi/monitor_p.cpp b/akonadi/monitor_p.cpp
index 6f51fc0..6b44efb 100644
--- a/akonadi/monitor_p.cpp
+++ b/akonadi/monitor_p.cpp
@@ -538,17 +538,13 @@ bool MonitorPrivate::emitNotification(const NotificationMessageV3 &msg)
Collection col;
Q_FOREACH (const NotificationMessageV2::Entity &entity, msg.entities()) {
col = collectionCache->retrieve(entity.id);
- if (col.isValid()) {
- if (emitCollectionNotification(msg, col, parent, destParent) && !someoneWasListening) {
- someoneWasListening = true;
- }
+ if (emitCollectionNotification(msg, col, parent, destParent) && !someoneWasListening) {
+ someoneWasListening = true;
}
}
} else if (msg.type() == NotificationMessageV2::Items) {
const Item::List items = itemCache->retrieve(msg.uids());
- if (!items.isEmpty()) {
- someoneWasListening = emitItemsNotification(msg, items, parent, destParent);
- }
+ someoneWasListening = emitItemsNotification(msg, items, parent, destParent);
}
}
commit c597b2b440def3960f9f4dd2a6912534216ae040
Author: Christian Mollekopf <chrigi_1 at fastmail.fm>
Date: Wed Oct 15 21:15:37 2014 +0200
Revert "Always assume someoneWasListening when we get invalid notification in emityNotification()"
This reverts commit afc24995c526e6f6ee3d231a21ce21bf08fb69d4.
someoneWasListening is essential for the change recorder and simply
setting this to true if no one was listening breaks it (because no one
will call replayNext).
Conflicts:
akonadi/monitor_p.cpp
diff --git a/akonadi/monitor_p.cpp b/akonadi/monitor_p.cpp
index 47605e7..6f51fc0 100644
--- a/akonadi/monitor_p.cpp
+++ b/akonadi/monitor_p.cpp
@@ -542,24 +542,12 @@ bool MonitorPrivate::emitNotification(const NotificationMessageV3 &msg)
if (emitCollectionNotification(msg, col, parent, destParent) && !someoneWasListening) {
someoneWasListening = true;
}
- } else {
- // We don't know if someone is actually listening, but we don't want
- // to trigger cleanOldNotifications() every time we run into an invalid
- // notification, because it's rather expensive
- someoneWasListening = true;
}
}
} else if (msg.type() == NotificationMessageV2::Items) {
const Item::List items = itemCache->retrieve(msg.uids());
if (!items.isEmpty()) {
someoneWasListening = emitItemsNotification(msg, items, parent, destParent);
- } else {
- // In case of a Remove notification this will return a list of invalid entities (we'll deal later with them)
- // Of course, we may end up with an inconsistency in the database and have a pending notification that has
- // been (e.g.) written out to a log for later replay (such as in a ChangeRecorder) and now references uids
- // that no longer exist. In *that* case we want to just drop the notification, otherwise we get stuck on it
- // forever
- someoneWasListening = msg.operation() == NotificationMessageV2::Remove;
}
}
}
More information about the commits
mailing list