D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich updated this revision to Diff 79751.
astippich added a comment.


  - rename signal

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28682?vs=79657=79751

BRANCH
  exportSignal

REVISION DETAIL
  https://phabricator.kde.org/D28682

AFFECTED FILES
  src/file/filecontentindexer.cpp
  src/file/filecontentindexer.h
  src/file/fileindexscheduler.cpp

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich added a comment.


  How about committedBatch so that it's clear that they have been committed to 
the database?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Stefan Brüns]

bruns added a comment.


  Regarding API, `done` is a little bit vague.
  
  Can you rename it to `finishedBatch` (or come up with a better name).

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Stefan Brüns]

bruns added a comment.


  In D28682#644914 , @astippich 
wrote:
  
  > Then it's fine from my application point of view. I can add a corresponding 
start signal for convenience, though. Your call.
  
  
  Lets keep the API as small as possible ...

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich added a comment.


  Then it's fine from my application point of view. I can add a corresponding 
start signal for convenience, though. Your call.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Stefan Brüns]

bruns added a comment.


  In D28682#644430 , @astippich 
wrote:
  
  > In D28682#644414 , @bruns wrote:
  >
  > > In general fine for me.
  > >
  > > How will Elisa deal with the indexer maybe crashing inbetween? Do we also 
need a signal for a batch start?
  >
  >
  > From my (limited) understanding, the finishedIndexingFile will no be 
emitted when an indexers crashes, does it? Then I think it is fine without an 
additional signal as long as the done() signal is still emitted as it is 
possible to reset the applications' internal list. Or will it retry some files 
of the current batch?
  
  
  It will not be emitted for the file it crashed on, but for the earlier files. 
So you may have the following for the files 'a, b, c, d':
  
  - S:a, F:a
  - S:b, F:b
  - S:c 
  - S:a, F:a
  - S:b, F:b
  - done
  - S:c, 
  - S:d, F:d
  - done
  
  So when you only listen to `finished` and `done`, you should keep the files 
in a `set`, but after the `done` the earlier files should be committed.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich added a comment.


  > Quoted Text
  > 
  >> ! In D28682#644412 , @mgallien 
wrote:
  > 
  > This is the reason why it is so slow in Elisa?
  
  Do you mean slow for picking up changes in files? Then yes. This happens when 
a track is modified when Elisa is running and listens for finishedIndexingFile 
signal. It then immediately queries baloo database, which still has the old 
values.
  Only after the done signal the transaction to baloo's database is complete.
  
  In D28682#644414 , @bruns wrote:
  
  > In general fine for me.
  >
  > How will Elisa deal with the indexer maybe crashing inbetween? Do we also 
need a signal for a batch start?
  
  
  From my (limited) understanding, the finishedIndexingFile will no be emitted 
when an indexers crashes, does it? Then I think it is fine without an 
additional signal as long as the done() signal is still emitted as it is 
possible to reset the applications' internal list. Or will it retry some files 
of the current batch?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Stefan Brüns]

bruns added a comment.


  In general fine for me.
  
  How will Elisa deal with the indexer maybe crashing inbetween? Do we also 
need a signal for a batch start?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Matthieu Gallien]

mgallien added a comment.


  This is the reason why it is so slow in Elisa?

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28682

To: astippich, #baloo, bruns
Cc: mgallien, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28682: export done signal in filecontentindexer

[Alexander Stippich]

astippich created this revision.
astippich added reviewers: Baloo, bruns.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  Applications can subsribe for new files being indexed by baloo
  via the dbus interface of fileindexer. However, currently only
  the start and finishedIndexing signals are exported.
  The latter does only indicate that the file has been indexed,
  but not yet committed to the database.  
  Export the done signal so that it is possible to know
  when the transaction has been committed and thus is safe to
  query the database for metadata.

REPOSITORY
  R293 Baloo

BRANCH
  exportSignal

REVISION DETAIL
  https://phabricator.kde.org/D28682

AFFECTED FILES
  src/file/filecontentindexer.h

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams