Add button to reset index database and repair Baloo crashing
Needs ReviewPublic

Authored by guoyunhe on Feb 10 2019, 12:13 AM.

Details

Reviewers
None
Group Reviewers
Plasma
Baloo
Summary

Here are a lot of bug reports of Baloo crashing caused by corrupted
database. This patch provide a button in KCM to quickly delete and rebuild index
database.

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8169
Build 8187: arc lint + arc unit
guoyunhe created this revision.Feb 10 2019, 12:13 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 10 2019, 12:13 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
guoyunhe requested review of this revision.Feb 10 2019, 12:13 AM
guoyunhe edited the summary of this revision. (Show Details)Feb 10 2019, 12:15 AM
guoyunhe added reviewers: Plasma, Baloo.
guoyunhe added a project: Baloo.

I wish this weren't necessary, but I think it's a good idea for the time being.

Can you reduce the button width so it doesn't span the entire layout? It should be as small as possible and right-aligned.

guoyunhe updated this revision to Diff 51324.Feb 10 2019, 4:27 PM
  • Reset index database button align right

I wish this weren't necessary, but I think it's a good idea for the time being.

Can you reduce the button width so it doesn't span the entire layout? It should be as small as possible and right-aligned.

I made it right aligned with smaller width.

Great! Just one more UI suggestion: use the string "Rebuild Index Database"

guoyunhe updated this revision to Diff 51325.Feb 10 2019, 4:37 PM
  • Reset index database button align right

Great! Just one more UI suggestion: use the string "Rebuild Index Database"

Updated :-)

Thanks! This is pretty good as-is, and I can confirm that it works just fine. However once the user presses the button, there's no further feedback, which could encourage them to repeatedly press it again--not a good idea. Maybe while the initial index is being generated, we could display a progress spinner (and a label too) and disable the button. What do you think?

kcms/baloo/kcm.cpp
209

This whole section could benefit from a few more line breaks to separate the commands into logical groups.

guoyunhe updated this revision to Diff 51435.Feb 11 2019, 7:05 PM
  • Add message box for Rebuild Index Database button

Screenshot:

guoyunhe updated this revision to Diff 51436.Feb 11 2019, 7:07 PM
  • Make message box translatable

Thanks!

  1. Could we use an inline KMessageWidget instead of a dialog box? Dialog boxes are annoying.
  2. The text should be something more like "Database is now being rebuilt", since if I'm understanding the code, it will be displayed immediately. So it isn't actually accurate to say that the database has already been rebuilt.
bruns added a subscriber: bruns.Feb 11 2019, 7:21 PM

Thanks! This is pretty good as-is, and I can confirm that it works just fine. However once the user presses the button, there's no further feedback, which could encourage them to repeatedly press it again--not a good idea. Maybe while the initial index is being generated, we could display a progress spinner (and a label too) and disable the button. What do you think?

No, please no progress dialog, a full rebuild can take hours ...

No, I don't want a dialog window of any kind. Just a spinner on the page that says, "Indexing is in progress" or something was what I had in mind. An inline KMessageWidget is okay too. However my major concern was communicating to the user that something happened so they don't click the button multiple times.

bruns added inline comments.Feb 11 2019, 7:24 PM
kcms/baloo/kcm.cpp
197

IMHO this whole block should be a single, blocking command in balooctl, i.e. move the whole logic to balooctl.

210

Care to elaborate?

guoyunhe added inline comments.Feb 11 2019, 7:32 PM
kcms/baloo/kcm.cpp
197

Will try.

210

If I run "balooctl start" in QProcess, baloo_file always fail to start. I didn't figure out the reason. But running baloo_file directly always works.

If we know the DB is corrupted, and it's just a cache, why do we need a user facing button?

If we know the DB is corrupted, and it's just a cache, why do we need a user facing button?

It is also possible to move this function to database corruption handler. One thing I worry about is: if the database corruption is caused by software reason, it will repeat the corrupt-rebuild-corrupt dead loop...

mart added a subscriber: mart.Feb 26 2019, 10:55 AM

If we know the DB is corrupted, and it's just a cache, why do we need a user facing button?

It is also possible to move this function to database corruption handler. One thing I worry about is: if the database corruption is caused by software reason, it will repeat the corrupt-rebuild-corrupt dead loop...

maybe also add an heuristic that if has been automatically rebuilt more than x times in the last y time, stop doing it?

In D18890#419886, @mart wrote:

If we know the DB is corrupted, and it's just a cache, why do we need a user facing button?

It is also possible to move this function to database corruption handler. One thing I worry about is: if the database corruption is caused by software reason, it will repeat the corrupt-rebuild-corrupt dead loop...

maybe also add an heuristic that if has been automatically rebuilt more than x times in the last y time, stop doing it?

Should work. I will make a patch for Baloo and then update this patch.

bruns added a comment.Feb 27 2019, 1:55 AM

If we know the DB is corrupted, and it's just a cache, why do we need a user facing button?

There are many possible ways of corruption:

  • non-decodable values
  • entries with dangling parent IDs
  • IDs of no longer existing documents
  • ....

These have different reasons, not all reasons are known and understood, not all corruptions are critical.

There are issues with races in the file system, where items are deleted/created/modified while these are added to the DB. Unfortunately, Qt offers no way to handle it, the only save method is to use the openat/fstatat/... functions. This is possible to fix, but until then, rinse and repeat.

The non-decodable values are hard to handle, this would at least require scrubbing the whole DB or building it from scratch. But until it is known what causes this corruption, we end up in doing it again and again.

Some of these are quite safe to just ignore, but some parts a lacking sanity checks. On the other hand, D12336 is waiting for review for 10 months ...