D16266: [Extractor] Make extractor crash resilient

2018-10-25 Thread Stefan Brüns
This revision was automatically updated to reflect the committed changes. Closed by commit R293:1509ca51c5ed: [Extractor] Make extractor crash resilient (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16266?vs=43777=44212 REVISION DETAIL

D16266: [Extractor] Make extractor crash resilient

2018-10-25 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Yeah go ahead I think. REPOSITORY R293 Baloo BRANCH mimetype_handling REVISION DETAIL https://phabricator.kde.org/D16266 To: bruns, #baloo, #frameworks, poboiko, ngraham Cc:

D16266: [Extractor] Make extractor crash resilient

2018-10-24 Thread Stefan Brüns
bruns marked an inline comment as done. bruns added inline comments. INLINE COMMENTS > poboiko wrote in filecontentindexer.cpp:91 > OK, we can simply count how many times we got `finishedIndexingFile`, and > just go to the corresponding position in the batch. > It's just the binary search here

D16266: [Extractor] Make extractor crash resilient

2018-10-20 Thread Igor Poboiko
poboiko added inline comments. INLINE COMMENTS > bruns wrote in filecontentindexer.cpp:91 > But it is racy - if the file is replaced in the meantime, inode and filename > no longer match. This is not completely unlikely when dealing with temporary > files. > > It would make the code also

D16266: [Extractor] Make extractor crash resilient

2018-10-17 Thread Stefan Brüns
bruns marked 3 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D16266 To: bruns, #baloo, #frameworks, poboiko, ngraham Cc: broulik, apol, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D16266: [Extractor] Make extractor crash resilient

2018-10-17 Thread Stefan Brüns
bruns marked 3 inline comments as done. bruns added inline comments. INLINE COMMENTS > poboiko wrote in filecontentindexer.cpp:91 > There is `Transaction::documentId(const QByteArray& path)`, which can resolve > it using `DocumentUrlDB`. > I believe it still would be cheaper than reindexing

D16266: [Extractor] Make extractor crash resilient

2018-10-17 Thread Igor Poboiko
poboiko added inline comments. INLINE COMMENTS > bruns wrote in filecontentindexer.cpp:91 > Yes, its a binary search. > Because we only have IDs here, and the progress reporting works on strings. There is `Transaction::documentId(const QByteArray& path)`, which can resolve it using

D16266: [Extractor] Make extractor crash resilient

2018-10-17 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > poboiko wrote in filecontentindexer.cpp:91 > Do I understand correctly, that's a binary-search-like way to find the file > actually causing `extractor` to crash, and it will reindex some of the files > in a batch ~log2(batchsize) times? > Why

D16266: [Extractor] Make extractor crash resilient

2018-10-17 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > poboiko wrote in filecontentindexer.cpp:75 > Is it OK to use `QObject::connect` inside a `while` loop? Those are not > `Qt::UniqueConnection`, won't they fire multiple times (more and more, > actually)? given `loop` lives on the stack it is

D16266: [Extractor] Make extractor crash resilient

2018-10-17 Thread Igor Poboiko
poboiko added inline comments. INLINE COMMENTS > filecontentindexer.cpp:75 > +bool hadErrors = false; > +connect(, ::failed, , [, > ]() { hadErrors = true; loop.quit(); }); > + Is it OK to use `QObject::connect` inside a `while` loop? Those are not `Qt::UniqueConnection`,

D16266: [Extractor] Make extractor crash resilient

2018-10-16 Thread Stefan Brüns
bruns updated this revision to Diff 43777. bruns added a comment. rebase REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16266?vs=43769=43777 BRANCH mimetype_handling REVISION DETAIL https://phabricator.kde.org/D16266 AFFECTED FILES

D16266: [Extractor] Make extractor crash resilient

2018-10-16 Thread Stefan Brüns
bruns updated this revision to Diff 43769. bruns marked 2 inline comments as done. bruns added a comment. coding style REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16266?vs=43768=43769 BRANCH mimetype_handling REVISION DETAIL

D16266: [Extractor] Make extractor crash resilient

2018-10-16 Thread Stefan Brüns
bruns marked 3 inline comments as done. bruns added inline comments. INLINE COMMENTS > apol wrote in extractorprocess.cpp:35 > Shouldn't it check the exitCode too? the exitCode is "only valid for normal exits" (Qt docu ), and 0 otherwise. > apol

D16266: [Extractor] Make extractor crash resilient

2018-10-16 Thread Aleix Pol Gonzalez
apol added inline comments. INLINE COMMENTS > extractorprocess.cpp:35 > +connect(_extractorProcess, QOverload QProcess::ExitStatus>::of(::finished), > +[=](int exitCode, QProcess::ExitStatus exitStatus) > +{ Shouldn't it check the exitCode too? >

D16266: [Extractor] Make extractor crash resilient

2018-10-16 Thread Stefan Brüns
bruns created this revision. bruns added reviewers: Baloo, Frameworks, poboiko, ngraham. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Connect to QProcess::finished to detect the exit status.