Separate UIDs and Revisions in main databases
ClosedPublic

Authored by rnicole on Aug 21 2018, 12:56 PM.

Details

Summary
  • Change revision type from qint64 to size_t for LMDB in a couple of places (LMDB supports unsigned int or size_t which are long unsigned int on my machine)
  • Better support for database flags (duplicate, integer keys, integer values for now but is extensible)
  • Main databases' keys are now revisions
  • Some databases switched to integer keys databases:
    • Main databases
    • the revision to uid mapping database
    • the revision to entity type mapping database
  • Refactor the entity type's typeDatabases method (if in the future we need to change the main databases' flags again)
  • New uid to revision mapping database (uidsToRevisions):
    • Stores all revisions (not uid to latest revision) because we need it for cleaning old revisions
    • Flags are: duplicates + integer values (so findLatest finds the latest revision for the given uid)

Problems to fix before merging:
All Fixed!

  • Sometimes Sink can't read what has just been written to the database (maybe because of transactions race conditions)
    • Most of the times, this results in Sink not able to find the uid for a given revision by reading the revisions database
  • pipelinetest's testModifyWithConflict fails because the local changes are overridden

The first problem prevents me from running benchmarks

Diff Detail

Repository
R9 Sink
Lint
Lint Skipped
Unit
Unit Tests Skipped
rnicole requested review of this revision.Aug 21 2018, 12:56 PM
rnicole created this revision.
cmollekopf added inline comments.Aug 21 2018, 2:02 PM
common/storage/entitystore.cpp
252

Printing and internalByteArray() probably doesn't make much sense. Since we already print identifier and revision I think this can be removed. Alternatively print the displayByteArray() and remove identifier and newRevision.

447

The purpose of this function is to lookup the latest revision of all uids available.

This should probably be rewritten to use one of the revision to uid indexes to generate that list of uids with their corresponding latest revision.

add a test to entitystoretest.cpp while you're at it.

common/storage_common.cpp
137–177

Move to top or remove.

common/storage_lmdb.cpp
428

Inline function call.

rnicole marked 3 inline comments as done.Aug 21 2018, 2:17 PM
rnicole updated this revision to Diff 40191.EditedAug 22 2018, 7:58 AM
rnicole edited the summary of this revision. (Show Details)
  • Fixed the random reading issues on database with integer keys with great help from Christian
  • Fixed the readPrevious function from the entity store
  • Fixed dumb issue in the size_t variant of the scan function
  • Add some storage tests

Still TODO:
Done!

  • Fix the last test of pipelinetest
  • Redo the fullScan function and test it
rnicole marked an inline comment as done.Aug 22 2018, 8:36 AM
rnicole updated this revision to Diff 40197.Aug 22 2018, 9:22 AM
rnicole edited the summary of this revision. (Show Details)

Fix the testModifyWithConflict from pipelinetest by fixing the readRevisions function which was still using UIDs to access a main database. Now it uses the uidsToRevisions database.

Benchmarks coming up

rnicole updated this revision to Diff 40215.Aug 22 2018, 11:48 AM
  • Fix model notifications by fixing the contains function
  • Add tests to the contains and exists functions from the Entity store
This revision was not accepted when it landed; it landed in state Needs Review.Aug 22 2018, 1:03 PM
This revision was automatically updated to reflect the committed changes.