Quit baloo_file_extractor if DB transaction fails
Needs ReviewPublic

Authored by davidedmundson on Nov 27 2019, 11:49 PM.

Details

Reviewers
bruns
ngraham
Summary

Currently I have a broken DB, the worst part is that I'm in a 100% CPU
loop.

baloo-file runs, sees some files need phase 2 extraction, and launches
the extractor. The extractor fails to update the DB which means they're
never marked as complete or failed.

baloo-file then keeps sending the same files

By having the extractor bail, it at least stops.

Test Plan

Ran baloo-file
It fails for 20 files or so (one batch) then stops

Diff Detail

Repository
R293 Baloo
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19242
Build 19260: arc lint + arc unit
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptNov 27 2019, 11:49 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
davidedmundson requested review of this revision.Nov 27 2019, 11:49 PM
meven added a subscriber: meven.Nov 28 2019, 10:25 AM
meven added inline comments.
src/file/extractor/app.cpp
145

Add a Warning/log message ? qCWarning(BALOO) << "Could not save data to DB" or similar

src/file/extractor/app.cpp
145

There's a tonne in the transaction code

meven added inline comments.Nov 28 2019, 10:32 AM
src/file/extractor/app.cpp
145

Alright but since the program exits here, I am concerned it may not be clear enough to the user/caller as to why it exited ?
Adding a log here will add a different context.
Also QCoreApplication::exit(-1);?

New output:

_TXN: Transaction must abort, has a child, or is invalid
12:35:09.167 org.kde.baloo.engine: Baloo::PositionDB::put|Baloo::WriteTransaction::commit|Baloo::Transaction::commit PositionDB::put MDB_BAD_TXN: Transaction must abort, has a child, or is invalid
12:35:09.171 org.kde.baloo.engine: Baloo::Transaction::commit|?baloo_file_extractor?|?baloo_file_extractor? Transaction::commit MDB_BAD_TXN: Transaction must abort, has a child, or is invalid
*20
12:35:09.172 Baloo::App::processNextFile|?baloo_file_extractor?|?baloo_file_extractor? Could not commit results. Quitting

ngraham accepted this revision.Nov 29 2019, 5:09 PM
This revision is now accepted and ready to land.Nov 29 2019, 5:09 PM
bruns requested changes to this revision.Nov 29 2019, 6:00 PM

Please allow some time for reviewing ...

This revision now requires changes to proceed.Nov 29 2019, 6:00 PM

Ack, was going to wait for you :)

davidedmundson requested review of this revision.Dec 18 2019, 10:29 AM
davidedmundson marked 3 inline comments as done.

Please allow some time for reviewing ...

Marking as request review, so it hopefully reappears in your TODO queue.

bruns added a comment.Jan 24 2020, 9:33 AM

I will try to cook up a more complete solution over the weekend.

src/file/extractor/app.cpp
119

This is a little bit confusing, as the lmdb code uses rc == LMDB_OK == 0, but here 0 == ERROR.

bruns added a comment.Jan 24 2020, 9:34 AM

... and sorry for the late answer ...

I will try to cook up a more complete solution over the weekend.

If you can explain what you would prefer instead, I can help find time.

src/file/extractor/app.cpp
119

Sure, but engine is an abstraction layer over lmdb so you don't want to leak those details, and for a boolean it makes sense that true == good

I'm going to push soon if I don't hear soon. It solves a real world problem and it's had one approval.

bruns added a comment.Feb 14 2020, 3:33 PM

From the viewpoint of baloo_file, the process just hangs, which is bad.

src/file/extractor/app.cpp
138

You still send out the message, though the results have not been commited.

From the viewpoint of baloo_file, the process just hangs, which is bad.

Ok, I can add support for passing the error down to baloo_file.