no way to wait for index to finish updating/(re)building

[sascha_silbe]

The datastore (re)builds the index in the background (idle loop), but has no API to wait for this to finish. That creates a race condition when using the datastore from a test suite:
The test suite needs to run from a clean directory every time (so it's deterministic), thus causing the datastore to do an index build on startup. The datastore is started automatically by DBus and the tests fire their API requests right away, so any API call that uses the index will fail to work as intended (for regular use).

[sascha_silbe]

add API to check and wait for index rebuild to finish

[sascha_silbe]

Implemented the check_ready() method and Ready() signal from my datastore redesign proposal. Also added a wait_ready() method that blocks the caller until the index (re)build has finished. pep8.py/pylint clean.

[tomeu]

Replying to sascha_silbe:

The test suite needs to run from a clean directory every time (so it's deterministic), thus causing the datastore to do an index build on startup.

But in order to be deterministic, you also need to have the same data and metadata, having the same index is not enough. So, why not just delete the whole DS, then adding entries with Create?

[sascha_silbe]

But in order to be deterministic, you also need to have the same data and metadata, having the same index is not enough. So, why not just delete the whole DS, then adding entries with Create?

That's exactly what I'm doing. The test suite creates a temporary directory and starts up a new datastore instance with HOME set to the temporary directory. But since an empty directory doesn't contain an index, the datastore will schedule an index (re)build. I need to wait for the index build to finish, otherwise API calls like get_uniquevalues() or find() won't work properly.

[tomeu]

Replying to sascha_silbe:

But in order to be deterministic, you also need to have the same data and metadata, having the same index is not enough. So, why not just delete the whole DS, then adding entries with Create?

That's exactly what I'm doing. The test suite creates a temporary directory and starts up a new datastore instance with HOME set to the temporary directory. But since an empty directory doesn't contain an index, the datastore will schedule an index (re)build. I need to wait for the index build to finish, otherwise API calls like get_uniquevalues() or find() won't work properly.

Right, but I was suggesting creating the index cia create() calls, instead of reindexing the whole directory. Is this so impractical for your use case to warrant new API?

[tomeu]

Turned this was actually a bug in the DS because the index was marked as rebuilding even when it didn't existed at startup and there were no entries. Fixed that bug, I understand that this new API is not needed any more.

[sascha_silbe]

Still relevant because if the index gets corrupted (e.g. due to unclean shutdown on a non-dat journalling filesystem) it will be rebuilt in the background and return different results than during normal operation.

The old data store apparently had a similar method called complete_indexing() (though "indexing" meant full-text extraction there). My data store rewrite proposal contains the same API as proposed here with slightly different semantics as well ("regular" API calls are disallowed until the data store has finished recovering, instead of returning bogus results).

Even though there are slight semantic differences their usage by a caller is the same (wait for data store to be ready to server reliable results). As it's currently not used by any caller (complete_indexing() is still in sugar.datastore.datastore, but not in carquinyol.datastore) it's a low-risk change and would ease transition to the version-enabled data store.

[tomeu]

Replying to sascha_silbe:

Still relevant because if the index gets corrupted (e.g. due to unclean shutdown on a non-dat journalling filesystem) it will be rebuilt in the background and return different results than during normal operation.

That's fine, but why add API if nobody will use it?

[sascha_silbe]

Because I intend for it to get used. Introducing the API now allows for independant, small changes in other code.

[tomeu]

Replying to sascha_silbe:

Because I intend for it to get used. Introducing the API now allows for independant, small changes in other code.

I really believe we should be putting our energies in issues that are real right now. I also don't think it's a good idea to design APIs when we don't have real needs.

[sascha_silbe]

Moved to 0.88 as discussed on IRC.

[erikos]

Any update on this one? Moving out as it is not critical.

[sascha_silbe]

Related: #1546 (migrating large data store causes DBus timeout)

[dnarvaez]

Agree with last tomeu comment.

[dnarvaez]

Milestone 0.90 deleted