Differential D15983

React to config updates inside indexer
ClosedPublic
Actions

Authored by poboiko on Oct 6 2018, 4:27 PM.

Details

Reviewers
bruns
ngraham
Group Reviewers
Baloo
Frameworks
Commits
R293:e58804bf9eb8: React to config updates inside indexer
Summary

In case a config was changed, we need to:

  1. Stop the ContentIndexer (bug 373430)
  2. Remove all scheduled indexing of files that should not be indexed
  3. Check for stale index entries. Those includes documents marked for indexing, that should be removed

(because of that we need to do it inside scheduleIndexing before content indexing)

  1. Finally, check for unindexed files (if new files were added)

(based on D15939: Perform checks for unindexed files and stale index entries on startup). Also note that IndexCleaner should clean all files, not just those inside includeFolders

BUG: 373430

Test Plan

Seems to be working. But it would be nice to test it more.

  1. Add large folder to config. baloo_file start indexing it.
  2. Add it to exclude list. baloo_file stops indexing it, and removes entries from DB.
  3. Remove it from exclude list. baloo_file starts indexing it again.

Diff Detail

Repository
R293 Baloo
Branch
arcpatch-D15983
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4930
Build 4948: arc lint + arc unit
poboiko requested review of this revision.Oct 6 2018, 4:27 PM
poboiko created this revision.
bruns added a subscriber: bruns.Oct 10 2018, 10:58 PM

Looks good in general, but have to think a little bit more about it ..

src/file/fileindexscheduler.cpp
127

Should read "This has to be above ...", also do the line break at ~ 80 chars

183

"Interrupt content indexer, to avoid indexing files that should not be indexed, bug 373430)"

191

replace these two lines by

m_checkStaleIndexEntries = true;
m_checkUnindexedFiles = true;
scheduleIndexing();
poboiko updated this revision to Diff 43446.Oct 12 2018, 8:05 AM

Address raised issues

Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptOct 12 2018, 8:05 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
poboiko marked 3 inline comments as done.Oct 12 2018, 8:05 AM
poboiko updated this revision to Diff 45451.Nov 14 2018, 1:11 PM

Rebase on master

poboiko updated this revision to Diff 45452.Nov 14 2018, 1:47 PM

It's a bad idea to removeRecursively starting from root of the tree (documentid 0).
If user has indexed /home/username folder, there is also an index entry for /home (that's how IdTreeDB works).
However, /home should not be indexed, according to checks (because it's not in includeFolders, while /home/username is)
This will lead to removeRecursively("/home") call, which will wipe index for /home/username as well.

Instead I propose to do checks based on includeFolders and excludeFolders
(which could have changed if config was changed and some directory was added to exclude list)

poboiko edited the summary of this revision. (Show Details)Mar 19 2019, 8:47 AM
poboiko added reviewers: bruns, ngraham.

Ping?

ngraham added a comment.Mar 19 2019, 9:15 AM

Can you rebase this on master again? Sorry for the radio silence. :(

poboiko added a comment.Mar 19 2019, 10:58 AM
In D15983#434247, @ngraham wrote:

Can you rebase this on master again? Sorry for the radio silence. :(

Nevermind, I also kinda forgot about this :)
Seems like this patch still applies to current master:

# git rebase master
Current branch arcpatch-D15983 is up to date.
ngraham accepted this revision.Mar 19 2019, 11:19 AM
This revision is now accepted and ready to land.Mar 19 2019, 11:19 AM
poboiko closed this revision.Mar 20 2019, 9:01 AM

Revision Contents
Changeset List

Diff 45452

View Options

src/file/fileindexscheduler.h

Loading...
View Options

src/file/fileindexscheduler.cpp

Loading...
View Options

src/file/indexcleaner.cpp

Loading...
View Options

src/file/mainhub.cpp

Loading...