<table><tr><td style="">michaelh added a subscriber: mgallien.<br />michaelh added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D11529">View Revision</a></tr></table><br /><div><div><p>Under the premise that I still have to learn about baloo's inner workings, here are some concerns:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">I'm not convinced, that index cleaning should be part of the dbus interface. Why should cleaning be done out of process? Do we really want any application to trigger database manipulation? Could elaborate your rationale please?</li>
<li class="remarkup-list-item">Which application is using baloo dbus interface anyway and which functions? <a href="https://phabricator.kde.org/p/mgallien/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@mgallien</a> How is elisa using it?</li>
<li class="remarkup-list-item">Purging stale entries is by far not enough to get the index in a good shape. As soon as removable drives or network shares are involved all sorts of weird things can happen. IndexCleaner is much too simple to account for that.</li>
<li class="remarkup-list-item">Looks like indexcleaner has been dead code until now. Have you read Vishesh's commit message? <a href="https://cgit.kde.org/baloo.git/commit/?id=ea2afe88b0c4299d7540e5b6c8b8e46858336f0c" class="remarkup-link" target="_blank" rel="noreferrer">https://cgit.kde.org/baloo.git/commit/?id=ea2afe88b0c4299d7540e5b6c8b8e46858336f0c</a> Do we have to be concerned about his note?</li>
</ul></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57791">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fileindexscheduler.cpp:140</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">m_purgeDeindexableFiles</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">auto</span> <span class="n">runnable</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">IndexCleaner</span><span class="p">(</span><span class="n">m_db</span><span class="p">,</span> <span class="n">m_config</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same as above</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57792">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fileindexscheduler.cpp:225</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa4000">void</span> <span class="n">FileIndexScheduler</span><span style="color: #aa2211">::</span><span class="n">purgeDeindexableFiles</span><span class="p">()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same as above</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57793">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fileindexscheduler.cpp:227</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">m_purgeDeindexableFiles</span> <span style="color: #aa2211">=</span> <span style="color: #304a96">true</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">scheduleIndexing</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same as above</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57797">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fileindexscheduler.h:83</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">Q_SCRIPTABLE</span> <span style="color: #aa4000">void</span> <span style="color: #004012">checkUnindexedFiles</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_SCRIPTABLE</span> <span style="color: #aa4000">void</span> <span style="color: #004012">purgeDeindexableFiles</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">Q_SCRIPTABLE</span> <span class="n">uint</span> <span style="color: #004012">getBatchSize</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Nitpick: We're removing db entries not files. Also every file is deindexable. Maybe 'purgeStaleEntries', 'removeLostEntries' or so would be a better name to describe what's going on.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57798">View Inline</a><span style="color: #4b4d51; font-weight: bold;">fileindexscheduler.h:110</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">bool</span> <span class="n">m_checkUnindexedFiles</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">bool</span> <span class="n">m_purgeDeindexableFiles</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span class="p">};</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same as above</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57794">View Inline</a><span style="color: #4b4d51; font-weight: bold;">indexcleaner.cpp:44</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">QMimeDatabase</span> <span class="n">mimeDb</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">Transaction</span> <span style="color: #004012">tr</span><span class="p">(</span><span class="n">m_db</span><span class="p">,</span> <span class="n">Transaction</span><span style="color: #aa2211">::</span><span class="n">ReadWrite</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Unrelated whitespace change</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57779">View Inline</a><span style="color: #4b4d51; font-weight: bold;">indexcleaner.cpp:80</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">for</span> <span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QString</span><span style="color: #aa2211">&</span> <span style="color: #a0a000">folder</span> <span class="p">:</span> <span class="n">m_config</span><span style="color: #aa2211">-></span><span class="n">includeFolders</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">folder</span><span class="p">.</span><span class="n">startsWith</span><span class="p">(</span><span class="n">device</span><span class="p">.</span><span class="n">mountPath</span><span class="p">()</span> <span style="color: #aa2211">+</span> <span style="color: #766510">"/"</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">quint64</span> <span class="n">id</span> <span style="color: #aa2211">=</span> <span class="n">filePathToId</span><span class="p">(</span><span class="n">QFile</span><span style="color: #aa2211">::</span><span class="n">encodeName</span><span class="p">(</span><span class="n">folder</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You can use <tt style="background: #ebebeb; font-size: 13px;">QStringLiteral("%1/").arg(device.mountPath())</tt> here and maybe define it outside the loop</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57795">View Inline</a><span style="color: #4b4d51; font-weight: bold;">main.cpp:85</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">parser</span><span class="p">.</span><span class="n">addPositionalArgument</span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"resume"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Resume the file indexer"</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">parser</span><span class="p">.</span><span class="n">addPositionalArgument</span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"check"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Check for <span class="bright">any unindex</span>ed f<span class="bright">iles and index them</span>"</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">parser</span><span class="p">.</span><span class="n">addPositionalArgument</span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"check"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Check for <span class="bright">changes in the monitor</span>ed f<span class="bright">olders</span>"</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">parser</span><span class="p">.</span><span class="n">addPositionalArgument</span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"index"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Index the specified files"</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Something like "Update the index. Remove stale entries and add new files." would describe the actions taken more clearly.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11529#inline-57790">View Inline</a><span style="color: #4b4d51; font-weight: bold;">main.cpp:197</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">out</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Started search for unindexed files</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">schedulerinterface</span><span class="p">.</span><span class="n">purgeDeindexableFiles</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">out</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Started search for deindexable files</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Do purging before indexing.<br />
Sometimes the parent id of a file is lost (Why?) and they appear as <tt style="background: #ebebeb; font-size: 13px;">//foo.bar</tt> if that is deleted first it can be reindexed.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R293 Baloo</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11529">https://phabricator.kde.org/D11529</a></div></div><br /><div><strong>To: </strong>smithjd, Baloo, vhanda, michaelh<br /><strong>Cc: </strong>mgallien, Frameworks, ashaposhnikov, michaelh, astippich, spoorun, nicolasfella, ngraham, alexeymin<br /></div>