summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Vrátil <dvratil@kde.org>2016-09-14 12:46:02 (GMT)
committerDaniel Vrátil <dvratil@kde.org>2016-09-14 12:46:49 (GMT)
commitc7944585b6e68e67d49e35d09cd148f1e1d5baed (patch)
tree709a1acbb200c36c41b7812595cb2852649e7e96
parent254a636c748857732ec26ee53b774d3935b569e2 (diff)
Reduce collection statistics queries
For certain cases we can now update the cached statistics in CollectionStatistics incrementally instead of invalidating the cache and querying DB to calculate it again. The calculation is rather expensive (one of the most expensive queries we have) and the time we spent recalculating the stats quickly adds up to secons. Currently we only support incremental stats update for flags change within a single collection and for appending new items which, especially in the second case, should notably reduce the amount of the super-heavy statistics queries. In any other case we currently fall-back to invalidating the cache and calculating the stats on demand.
-rw-r--r--autotests/server/CMakeLists.txt1
-rw-r--r--autotests/server/collectionstatisticstest.cpp159
-rw-r--r--autotests/server/fakedatastore.cpp3
-rw-r--r--autotests/server/fakedatastore.h1
-rw-r--r--src/server/akonadi.cpp1
-rw-r--r--src/server/handler/akappend.cpp7
-rw-r--r--src/server/handler/akappend.h2
-rw-r--r--src/server/handler/copy.cpp8
-rw-r--r--src/server/storage/collectionstatistics.cpp48
-rw-r--r--src/server/storage/collectionstatistics.h11
-rw-r--r--src/server/storage/datastore.cpp38
-rw-r--r--src/server/storage/datastore.h1
-rw-r--r--src/server/storage/notificationcollector.cpp17
-rw-r--r--src/server/storage/notificationcollector.h3
14 files changed, 278 insertions, 22 deletions
diff --git a/autotests/server/CMakeLists.txt b/autotests/server/CMakeLists.txt
index 78f99e6..a6a5ebc 100644
--- a/autotests/server/CMakeLists.txt
+++ b/autotests/server/CMakeLists.txt
@@ -86,6 +86,7 @@ add_server_test(parthelpertest.cpp)
add_server_test(itemretrievertest.cpp)
add_server_test(notificationmanagertest.cpp)
add_server_test(parttypehelpertest.cpp)
+add_server_test(collectionstatisticstest.cpp)
if (SQLITE_FOUND) # tests using the fake server need the QSQLITE3 plugin
add_server_test(partstreamertest.cpp)
diff --git a/autotests/server/collectionstatisticstest.cpp b/autotests/server/collectionstatisticstest.cpp
new file mode 100644
index 0000000..7f6a01f
--- /dev/null
+++ b/autotests/server/collectionstatisticstest.cpp
@@ -0,0 +1,159 @@
+/*
+ * Copyright (C) 2016 Daniel Vrátil <dvratil@kde.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <QObject>
+
+#include "storage/collectionstatistics.h"
+#include "fakeakonadiserver.h"
+#include "dbinitializer.h"
+#include "aktest.h"
+
+using namespace Akonadi::Server;
+
+
+class IntrospectableCollectionStatistics : public CollectionStatistics
+{
+public:
+ IntrospectableCollectionStatistics()
+ : CollectionStatistics()
+ , mCalculationsCount(0)
+ {}
+ ~IntrospectableCollectionStatistics()
+ {}
+
+ int calculationsCount() const
+ {
+ return mCalculationsCount;
+ }
+
+protected:
+ Statistics calculateCollectionStatistics(const Collection &col) Q_DECL_OVERRIDE
+ {
+ ++mCalculationsCount;
+ return CollectionStatistics::calculateCollectionStatistics(col);
+ }
+
+private:
+ int mCalculationsCount;
+};
+
+
+class CollectionStatisticsTest : public QObject
+{
+ Q_OBJECT
+
+public:
+ CollectionStatisticsTest()
+ {
+ try {
+ FakeAkonadiServer::instance()->setPopulateDb(false);
+ FakeAkonadiServer::instance()->init();
+ } catch (const FakeAkonadiServerException &e) {
+ qWarning() << "Server exception: " << e.what();
+ qFatal("Fake Akonadi Server failed to start up, aborting test");
+ }
+ }
+
+ ~CollectionStatisticsTest()
+ {
+ FakeAkonadiServer::instance()->quit();
+ }
+
+private Q_SLOTS:
+ void testCalculateStats()
+ {
+ DbInitializer initializer;
+ initializer.createResource("testresource");
+ auto col = initializer.createCollection("col1");
+ initializer.createItem("item1", col);
+ initializer.createItem("item2", col);
+ initializer.createItem("item3", col);
+
+ IntrospectableCollectionStatistics cs;
+ auto stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 3);
+ QCOMPARE(stats.read, 0);
+ QCOMPARE(stats.size, 0);
+ }
+
+ void testSeenChanged()
+ {
+ DbInitializer initializer;
+ initializer.createResource("testresource");
+ auto col = initializer.createCollection("col1");
+ initializer.createItem("item1", col);
+ initializer.createItem("item2", col);
+ initializer.createItem("item3", col);
+
+ IntrospectableCollectionStatistics cs;
+ auto stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 3);
+ QCOMPARE(stats.read, 0);
+ QCOMPARE(stats.size, 0);
+
+ cs.itemsSeenChanged(col, 2);
+ stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 3);
+ QCOMPARE(stats.read, 2);
+ QCOMPARE(stats.size, 0);
+
+ cs.itemsSeenChanged(col, -1);
+ stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 3);
+ QCOMPARE(stats.read, 1);
+ QCOMPARE(stats.size, 0);
+ }
+
+void testItemAdded()
+{
+ DbInitializer initializer;
+ initializer.createResource("testresource");
+ auto col = initializer.createCollection("col1");
+ initializer.createItem("item1", col);
+
+ IntrospectableCollectionStatistics cs;
+ auto stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 1);
+ QCOMPARE(stats.read, 0);
+ QCOMPARE(stats.size, 0);
+
+ cs.itemAdded(col, 5, true);
+ stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 2);
+ QCOMPARE(stats.read, 1);
+ QCOMPARE(stats.size, 5);
+
+ cs.itemAdded(col, 3, false);
+ stats = cs.statistics(col);
+ QCOMPARE(cs.calculationsCount(), 1);
+ QCOMPARE(stats.count, 3);
+ QCOMPARE(stats.read, 1);
+ QCOMPARE(stats.size, 8);
+ }
+};
+
+AKTEST_MAIN(CollectionStatisticsTest)
+
+#include "collectionstatisticstest.moc"
diff --git a/autotests/server/fakedatastore.cpp b/autotests/server/fakedatastore.cpp
index 264bc52..b13cf5f 100644
--- a/autotests/server/fakedatastore.cpp
+++ b/autotests/server/fakedatastore.cpp
@@ -233,6 +233,7 @@ void FakeDataStore::activeCachePolicy(Collection &col)
}
bool FakeDataStore::appendPimItem(QVector<Part> &parts,
+ const QVector<Flag> &flags,
const MimeType &mimetype,
const Collection &collection,
const QDateTime &dateTime,
@@ -248,7 +249,7 @@ bool FakeDataStore::appendPimItem(QVector<Part> &parts,
<< remote_id
<< remoteRevision
<< gid);
- return DataStore::appendPimItem(parts, mimetype, collection, dateTime,
+ return DataStore::appendPimItem(parts, flags, mimetype, collection, dateTime,
remote_id, remoteRevision, gid, pimItem);
}
diff --git a/autotests/server/fakedatastore.h b/autotests/server/fakedatastore.h
index 2dabe91..224ae50 100644
--- a/autotests/server/fakedatastore.h
+++ b/autotests/server/fakedatastore.h
@@ -91,6 +91,7 @@ public:
virtual void activeCachePolicy(Collection &col) Q_DECL_OVERRIDE;
virtual bool appendPimItem(QVector<Part> &parts,
+ const QVector<Flag> &flags,
const MimeType &mimetype,
const Collection &collection,
const QDateTime &dateTime,
diff --git a/src/server/akonadi.cpp b/src/server/akonadi.cpp
index fb80eb0..4ee1130 100644
--- a/src/server/akonadi.cpp
+++ b/src/server/akonadi.cpp
@@ -315,6 +315,7 @@ bool AkonadiServer::quit()
// Terminate the preprocessor manager before the database but after all connections are gone
PreprocessorManager::done();
+ CollectionStatistics::destroy();
if (DbConfig::isConfigured()) {
if (DataStore::hasDataStore()) {
diff --git a/src/server/handler/akappend.cpp b/src/server/handler/akappend.cpp
index 0585a47..10f7db2 100644
--- a/src/server/handler/akappend.cpp
+++ b/src/server/handler/akappend.cpp
@@ -146,7 +146,8 @@ bool AkAppend::insertItem(const Protocol::CreateItemCommand &cmd, PimItem &item,
PartHelper::insert(&hiddenAttribute);
}
- notify(item, item.collection());
+ const bool seen = flags.contains(AKONADI_FLAG_SEEN) || flags.contains(AKONADI_FLAG_IGNORED);
+ notify(item, seen, item.collection());
sendResponse(item, Protocol::CreateItemCommand::None);
return true;
@@ -336,9 +337,9 @@ bool AkAppend::sendResponse(const PimItem& item, Protocol::CreateItemCommand::Me
}
-bool AkAppend::notify(const PimItem &item, const Collection &collection)
+bool AkAppend::notify(const PimItem &item, bool seen, const Collection &collection)
{
- DataStore::self()->notificationCollector()->itemAdded(item, collection);
+ DataStore::self()->notificationCollector()->itemAdded(item, seen, collection);
if (PreprocessorManager::instance()->isActive()) {
// enqueue the item for preprocessing
diff --git a/src/server/handler/akappend.h b/src/server/handler/akappend.h
index 26340dc..9b176c0 100644
--- a/src/server/handler/akappend.h
+++ b/src/server/handler/akappend.h
@@ -55,7 +55,7 @@ private:
bool sendResponse(const PimItem &item, Protocol::CreateItemCommand::MergeModes mergeModes);
- bool notify(const PimItem &item, const Collection &collection);
+ bool notify(const PimItem &item, bool seen, const Collection &collection);
bool notify(const PimItem &item, const Collection &collection,
const QSet<QByteArray> &changedParts);
};
diff --git a/src/server/handler/copy.cpp b/src/server/handler/copy.cpp
index c9828d2..45d5a30 100644
--- a/src/server/handler/copy.cpp
+++ b/src/server/handler/copy.cpp
@@ -54,14 +54,10 @@ bool Copy::copyItem(const PimItem &item, const Collection &target)
}
DataStore *store = connection()->storageBackend();
- if (!store->appendPimItem(parts, item.mimeType(), target, QDateTime::currentDateTime(), QString(), QString(), item.gid(), newItem)) {
+ if (!store->appendPimItem(parts, item.flags(), item.mimeType(), target, QDateTime::currentDateTime(), QString(), QString(), item.gid(), newItem)) {
return false;
}
- Q_FOREACH (const Flag &flag, item.flags()) {
- if (!newItem.addFlag(flag)) {
- return false;
- }
- }
+
return true;
}
diff --git a/src/server/storage/collectionstatistics.cpp b/src/server/storage/collectionstatistics.cpp
index 37b8a3f..cc67002 100644
--- a/src/server/storage/collectionstatistics.cpp
+++ b/src/server/storage/collectionstatistics.cpp
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2014 Daniel Vrátil <dvratil@redhat.com>
+ * Copyright (C) 2016 Daniel Vrátil <dvratil@kde.org>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -22,6 +23,7 @@
#include "countquerybuilder.h"
#include "akonadiserver_debug.h"
#include "entities.h"
+#include "datastore.h"
#include <private/protocol_p.h>
@@ -39,8 +41,50 @@ CollectionStatistics *CollectionStatistics::self()
return sInstance;
}
+void CollectionStatistics::destroy()
+{
+ delete sInstance;
+ sInstance = Q_NULLPTR;
+}
+
+void CollectionStatistics::itemAdded(const Collection &col, qint64 size, bool seen)
+{
+ if (!col.isValid()) {
+ return;
+ }
+
+ QMutexLocker lock(&mCacheLock);
+ auto stats = mCache.find(col.id());
+ if (stats != mCache.end()) {
+ ++(stats->count);
+ stats->size += size;
+ stats->read += (seen ? 1 : 0);
+ } else {
+ mCache.insert(col.id(), calculateCollectionStatistics(col));
+ }
+}
+
+void CollectionStatistics::itemsSeenChanged(const Collection &col, qint64 seenCount)
+{
+ if (!col.isValid()) {
+ return;
+ }
+
+ QMutexLocker lock(&mCacheLock);
+ auto stats = mCache.find(col.id());
+ if (stats != mCache.end()) {
+ stats->read += seenCount;
+ } else {
+ mCache.insert(col.id(), calculateCollectionStatistics(col));
+ }
+}
+
void CollectionStatistics::invalidateCollection(const Collection &col)
{
+ if (!col.isValid()) {
+ return;
+ }
+
QMutexLocker lock(&mCacheLock);
mCache.remove(col.id());
}
@@ -50,12 +94,12 @@ const CollectionStatistics::Statistics CollectionStatistics::statistics(const Co
QMutexLocker lock(&mCacheLock);
auto it = mCache.find(col.id());
if (it == mCache.end()) {
- it = mCache.insert(col.id(), getCollectionStatistics(col));
+ it = mCache.insert(col.id(), calculateCollectionStatistics(col));
}
return it.value();
}
-CollectionStatistics::Statistics CollectionStatistics::getCollectionStatistics(const Collection &col)
+CollectionStatistics::Statistics CollectionStatistics::calculateCollectionStatistics(const Collection &col)
{
static const QString SeenFlagsTableName = QStringLiteral("SeenFlags");
static const QString IgnoredFlagsTableName = QStringLiteral("IgnoredFlags");
diff --git a/src/server/storage/collectionstatistics.h b/src/server/storage/collectionstatistics.h
index 031d897..77a3922 100644
--- a/src/server/storage/collectionstatistics.h
+++ b/src/server/storage/collectionstatistics.h
@@ -52,12 +52,19 @@ public:
};
static CollectionStatistics *self();
+ static void destroy();
+
+ virtual ~CollectionStatistics() {}
const Statistics statistics(const Collection &col);
+
+ void itemAdded(const Collection &col, qint64 size, bool seen);
+ void itemsSeenChanged(const Collection &col, qint64 seenCount);
+
void invalidateCollection(const Collection &col);
-private:
- Statistics getCollectionStatistics(const Collection &col);
+protected:
+ virtual Statistics calculateCollectionStatistics(const Collection &col);
QMutex mCacheLock;
QHash<qint64, Statistics> mCache;
diff --git a/src/server/storage/datastore.cpp b/src/server/storage/datastore.cpp
index 37beca1..dbc7f58 100644
--- a/src/server/storage/datastore.cpp
+++ b/src/server/storage/datastore.cpp
@@ -230,13 +230,14 @@ bool DataStore::hasDataStore()
/* --- ItemFlags ----------------------------------------------------- */
bool DataStore::setItemsFlags(const PimItem::List &items, const QVector<Flag> &flags,
- bool *flagsChanged, const Collection &col, bool silent)
+ bool *flagsChanged, const Collection &col_, bool silent)
{
QSet<QByteArray> removedFlags;
QSet<QByteArray> addedFlags;
QVariantList insIds;
QVariantList insFlags;
Query::Condition delConds(Query::Or);
+ Collection col = col_;
setBoolPtr(flagsChanged, false);
@@ -259,6 +260,12 @@ bool DataStore::setItemsFlags(const PimItem::List &items, const QVector<Flag> &f
insFlags << flag.id();
}
}
+
+ if (col.id() == -1) {
+ col.setId(item.collectionId());
+ } else if (col.id() != item.collectionId()) {
+ col.setId(-2);
+ }
}
if (!removedFlags.empty()) {
@@ -289,9 +296,10 @@ bool DataStore::setItemsFlags(const PimItem::List &items, const QVector<Flag> &f
}
bool DataStore::doAppendItemsFlag(const PimItem::List &items, const Flag &flag,
- const QSet<Entity::Id> &existing, const Collection &col,
+ const QSet<Entity::Id> &existing, const Collection &col_,
bool silent)
{
+ Collection col = col_;
QVariantList flagIds;
QVariantList appendIds;
PimItem::List appendItems;
@@ -303,6 +311,12 @@ bool DataStore::doAppendItemsFlag(const PimItem::List &items, const Flag &flag,
flagIds << flag.id();
appendIds << item.id();
appendItems << item;
+
+ if (col.id() == -1) {
+ col.setId(item.collectionId());
+ } else if (col.id() != item.collectionId()) {
+ col.setId(-2);
+ }
}
if (appendItems.isEmpty()) {
@@ -383,8 +397,9 @@ bool DataStore::appendItemsFlags(const PimItem::List &items, const QVector<Flag>
}
bool DataStore::removeItemsFlags(const PimItem::List &items, const QVector<Flag> &flags,
- bool *flagsChanged, const Collection &col, bool silent)
+ bool *flagsChanged, const Collection &col_, bool silent)
{
+ Collection col = col_;
QSet<QByteArray> removedFlags;
QVariantList itemsIds;
QVariantList flagsIds;
@@ -394,6 +409,11 @@ bool DataStore::removeItemsFlags(const PimItem::List &items, const QVector<Flag>
Q_FOREACH (const PimItem &item, items) {
itemsIds << item.id();
+ if (col.id() == -1) {
+ col.setId(item.collectionId());
+ } else if (col.id() != item.collectionId()) {
+ col.setId(-2);
+ }
for (int i = 0; i < flags.count(); ++i) {
const QByteArray flagName = flags[i].name().toLatin1();
if (!removedFlags.contains(flagName)) {
@@ -1015,6 +1035,7 @@ QMap<Entity::Id, QList<PimItem> > DataStore::virtualCollections(const PimItem::L
/* --- PimItem ------------------------------------------------------- */
bool DataStore::appendPimItem(QVector<Part> &parts,
+ const QVector<Flag> &flags,
const MimeType &mimetype,
const Collection &collection,
const QDateTime &dateTime,
@@ -1061,9 +1082,18 @@ bool DataStore::appendPimItem(QVector<Part> &parts,
}
}
+ bool seen = false;
+ Q_FOREACH (const Flag &flag, flags) {
+ seen |= (flag.name() == QLatin1String(AKONADI_FLAG_SEEN)
+ || flag.name() == QLatin1String(AKONADI_FLAG_IGNORED));
+ if (!pimItem.addFlag(flag)) {
+ return false;
+ }
+ }
+
// qCDebug(AKONADISERVER_LOG) << "appendPimItem: " << pimItem;
- mNotificationCollector->itemAdded(pimItem, collection);
+ mNotificationCollector->itemAdded(pimItem, seen, collection);
return true;
}
diff --git a/src/server/storage/datastore.h b/src/server/storage/datastore.h
index f8594c5..840fb79 100644
--- a/src/server/storage/datastore.h
+++ b/src/server/storage/datastore.h
@@ -178,6 +178,7 @@ public:
/* --- PimItem ------------------------------------------------------- */
virtual bool appendPimItem(QVector<Part> &parts,
+ const QVector<Flag> &flags,
const MimeType &mimetype,
const Collection &collection,
const QDateTime &dateTime,
diff --git a/src/server/storage/notificationcollector.cpp b/src/server/storage/notificationcollector.cpp
index 63e0120..1b50f83 100644
--- a/src/server/storage/notificationcollector.cpp
+++ b/src/server/storage/notificationcollector.cpp
@@ -52,10 +52,12 @@ NotificationCollector::~NotificationCollector()
}
void NotificationCollector::itemAdded(const PimItem &item,
+ bool seen,
const Collection &collection,
const QByteArray &resource)
{
SearchManager::instance()->scheduleSearchUpdate();
+ CollectionStatistics::self()->itemAdded(collection, item.size(), seen);
itemNotification(Protocol::ItemChangeNotification::Add, item, collection, Collection(), resource);
}
@@ -74,6 +76,10 @@ void NotificationCollector::itemsFlagsChanged(const PimItem::List &items,
const Collection &collection,
const QByteArray &resource)
{
+ int seenCount = (addedFlags.contains(AKONADI_FLAG_SEEN) || addedFlags.contains(AKONADI_FLAG_IGNORED) ? items.count() : 0);
+ seenCount -= (removedFlags.contains(AKONADI_FLAG_SEEN) || removedFlags.contains(AKONADI_FLAG_IGNORED) ? -items.count() : 0);
+
+ CollectionStatistics::self()->itemsSeenChanged(collection, seenCount);
itemNotification(Protocol::ItemChangeNotification::ModifyFlags, items, collection, Collection(), resource, QSet<QByteArray>(), addedFlags, removedFlags);
}
@@ -143,7 +149,9 @@ void NotificationCollector::collectionChanged(const Collection &collection,
if (AkonadiServer::instance()->intervalChecker()) {
AkonadiServer::instance()->intervalChecker()->collectionChanged(collection.id());
}
- CollectionStatistics::self()->invalidateCollection(collection);
+ if (changes.contains(AKONADI_PARAM_ENABLED) || changes.contains(AKONADI_PARAM_REFERENCED)) {
+ CollectionStatistics::self()->invalidateCollection(collection);
+ }
collectionNotification(Protocol::CollectionChangeNotification::Modify, collection, collection.parentId(), -1, resource, changes.toSet());
}
@@ -345,7 +353,12 @@ void NotificationCollector::itemNotification(Protocol::ItemChangeNotification::O
}
msg.setResource(res);
- CollectionStatistics::self()->invalidateCollection(col);
+ // Add and ModifyFlags are handled incrementally
+ // (see itemAdded() and itemsFlagsChanged())
+ if (msg.operation() != Protocol::ItemChangeNotification::Add
+ && msg.operation() != Protocol::ItemChangeNotification::ModifyFlags) {
+ CollectionStatistics::self()->invalidateCollection(col);
+ }
dispatchNotification(msg);
}
diff --git a/src/server/storage/notificationcollector.h b/src/server/storage/notificationcollector.h
index 536f1a6..c8622d7 100644
--- a/src/server/storage/notificationcollector.h
+++ b/src/server/storage/notificationcollector.h
@@ -71,7 +71,8 @@ public:
Provide as many parameters as you have at hand currently, everything
that is missing will be looked up in the database later.
*/
- void itemAdded(const PimItem &item, const Collection &collection = Collection(),
+ void itemAdded(const PimItem &item, bool seen,
+ const Collection &collection = Collection(),
const QByteArray &resource = QByteArray());
/**