Differential D13066

Add findAllInRange function in the storage layer
ClosedPublic
Actions

Authored by rnicole on May 23 2018, 3:27 PM.

Details

Reviewers
cmollekopf
Commits
R9:0456cef80e90: Add findAllInRange function in the storage layer
Summary

In preparation of the support for ranged queries.

Notes:

Since they are pretty similar, it could be nice to refactor scan and findAllInRange to use common 3rd function

Test Plan

This is tested in storagetest.cpp

Diff Detail

Repository
R9 Sink
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rnicole requested review of this revision.May 23 2018, 3:27 PM
rnicole created this revision.
cmollekopf accepted this revision.May 24 2018, 8:28 AM
This revision is now accepted and ready to land.May 24 2018, 8:28 AM
cmollekopf requested changes to this revision.May 24 2018, 8:41 AM

The tests crash:

Thread 1 "storagetest" received signal SIGILL, Illegal instruction.
0x00007ffff7009c33 in Sink::Storage::DataStore::NamedDatabase::findAllInRange(QByteArray const&, QByteArray const&, std::function<void (QByteArray const&, QByteArray const&)> const&, std::function<void (Sink::Storage::DataStore::Error const&)> const&) const (this=0x7fffffffdf38, lowerBound="0002" = {...},
    upperBound="0004" = {...}, resultHandler=..., errorHandler=...) at /src/sink/common/storage_lmdb.cpp:613
613         } while (mdb_cursor_get(cursor, &currentKey, &reference, MDB_NEXT) == MDB_SUCCESS);
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-24.fc27.x86_64 cyrus-sasl-lib-2.1.26-34.fc27.x86_64 dbus-libs-1.12.0-1.fc27.x86_64 freetype-2.8-8.fc27.x86_64 gamin-0.1.10-27.fc27.x86_64 glib2-2.54.3-2.fc27.x86_64 graphite2-1.3.10-3.fc27.x86_64 harfbuzz-1.4.8-1.fc27.x86_64 http-parser-2.7.1-7.fc27.x86_64 keyutils-libs-1.5.10-3.fc27.x86_64 kf5-kcalendarcore-17.12.3-1.fc27.x86_64 kf5-kcodecs-5.44.0-1.fc27.x86_64 kf5-kconfig-core-5.44.0-1.fc27.x86_64 kf5-kcontacts-17.12.3-1.fc27.x86_64 kf5-kcoreaddons-5.44.0-2.fc27.x86_64 kf5-ki18n-5.44.0-1.fc27.x86_64 kf5-kmime-17.12.3-1.fc27.x86_64 krb5-libs-1.15.2-7.fc27.x86_64 libX11-1.6.5-4.fc27.x86_64 libXau-1.0.8-9.fc27.x86_64 libXext-1.3.3-7.fc27.x86_64 libcom_err-1.43.5-2.fc27.x86_64 libcrypt-nss-2.26-27.fc27.x86_64 libcurl-7.55.1-10.fc27.x86_64 libgcc-7.3.1-5.fc27.x86_64 libgcrypt-1.8.2-1.fc27.x86_64 libgit2-0.26.3-1.fc27.x86_64 libglvnd-1.0.0-1.fc27.x86_64 libglvnd-glx-1.0.0-1.fc27.x86_64 libgpg-error-1.27-3.fc27.x86_64 libical-2.0.0-13.fc27.x86_64 libicu-57.1-9.fc27.x86_64 libidn2-2.0.4-3.fc27.x86_64 libnghttp2-1.25.0-1.fc27.x86_64 libpng-1.6.31-1.fc27.x86_64 libpsl-0.18.0-1.fc27.x86_64 libselinux-2.7-3.fc27.x86_64 libssh2-1.8.0-5.fc27.x86_64 libstdc++-7.3.1-5.fc27.x86_64 libunistring-0.9.9-1.fc27.x86_64 libuuid-2.30.2-2.fc27.x86_64 libxcb-1.12-5.fc27.x86_64 lmdb-libs-0.9.21-3.fc27.x86_64 lz4-libs-1.8.0-1.fc27.x86_64 nspr-4.18.0-1.fc27.x86_64 nss-3.35.0-1.1.fc27.x86_64 nss-softokn-freebl-3.35.0-1.0.fc27.x86_64 nss-util-3.35.0-1.0.fc27.x86_64 openldap-2.4.45-4.fc27.x86_64 openssl-libs-1.1.0g-1.fc27.x86_64 pcre-8.41-6.fc27.x86_64 pcre2-10.31-3.fc27.x86_64 pcre2-utf16-10.31-3.fc27.x86_64 qt5-qtbase-5.9.4-4.fc27.x86_64 qt5-qtbase-gui-5.9.4-4.fc27.x86_64 systemd-libs-234-10.git5f8984e.fc27.x86_64 xapian-core-libs-1.4.5-1.fc27.x86_64 xz-libs-5.2.3-4.fc27.x86_64 zlib-1.2.11-4.fc27.x86_64
(gdb) bt
#0  0x00007ffff7009c33 in Sink::Storage::DataStore::NamedDatabase::findAllInRange(QByteArray const&, QByteArray const&, std::function<void (QByteArray const&, QByteArray const&)> const&, std::function<void (Sink::Storage::DataStore::Error const&)> const&) const (this=0x7fffffffdf38, lowerBound="0002" = {...},
    upperBound="0004" = {...}, resultHandler=..., errorHandler=...) at /src/sink/common/storage_lmdb.cpp:613
#1  0x000000000042f8f5 in StorageTest::testFindRangeOptimistic (this=0x7fffffffea28) at /src/sink/tests/storagetest.cpp:480
#2  0x0000000000426099 in StorageTest::qt_static_metacall (_o=0x7fffffffea28, _c=QMetaObject::InvokeMetaMethod, _id=26, _a=0x7fffffffe160)
    at /build/sink/tests/storagetest_autogen/include/storagetest.moc:222
#3  0x00007ffff53c5515 in QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const () from /lib64/libQt5Core.so.5
#4  0x00007ffff76fb3ca in QTest::TestMethods::invokeTestOnData(int) const () from /lib64/libQt5Test.so.5
#5  0x00007ffff76fc120 in QTest::TestMethods::invokeTest(int, char const*, QTest::WatchDog*) const () from /lib64/libQt5Test.so.5
#6  0x00007ffff76fc691 in QTest::TestMethods::invokeTests(QObject*) const () from /lib64/libQt5Test.so.5
#7  0x00007ffff76fcc9b in QTest::qExec(QObject*, int, char**) () from /lib64/libQt5Test.so.5
#8  0x0000000000425e26 in main (argc=1, argv=0x7fffffffeb68) at /src/sink/tests/storagetest.cpp:733
This revision now requires changes to proceed.May 24 2018, 8:41 AM
cmollekopf added inline comments.May 24 2018, 8:43 AM
common/storage_lmdb.cpp
585

Rename reference to data. Just like in the other reading functions.

cmollekopf added inline comments.May 24 2018, 8:45 AM
common/storage_lmdb.cpp
582

One definition per line please (and including the type each time).

cmollekopf added inline comments.May 24 2018, 8:52 AM
common/storage_lmdb.cpp
588

I don't think looking up the last key is necessary. If you just use mdb_cmp you should be able to position the cursor at the first value, and then read until you either run out of values or the value is reported to be larger than what you are looking for.

cmollekopf added inline comments.May 24 2018, 8:53 AM
common/storage.h
111

Document that the boundaries are inclusive.

cmollekopf added inline comments.May 24 2018, 8:55 AM
common/storage_lmdb.cpp
606

Close the cursor.

621

Close the cursor with mdb_cursor_close(cursor); at the end.

rnicole updated this revision to Diff 34790.May 24 2018, 9:55 AM

Remove the ahead-of-time finding of the closest key to the upper bound

rnicole updated this revision to Diff 34794.May 24 2018, 10:02 AM
rnicole marked 6 inline comments as done.

Nitpicks

This revision was not accepted when it landed; it landed in state Needs Review.May 24 2018, 10:26 AM
Closed by commit R9:0456cef80e90: Add findAllInRange function in the storage layer (authored by rnicole, committed by cmollekopf). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcommon/storage.h (9 lines)
Mcommon/storage_lmdb.cpp (54 lines)
Mtests/storagetest.cpp (76 lines)

Diff 34798

View Options

common/storage.h

Loading...
View Options

common/storage_lmdb.cpp

Loading...
View Options

tests/storagetest.cpp

Loading...