Differential D21672

[PowerStateMonitor] Be conservative when determining power state
ClosedPublic
Actions

Authored by bruns on Jun 8 2019, 5:03 PM.

Details

Reviewers
ngraham
astippich
poboiko
Group Reviewers
Baloo
Commits
R293:0e6f1c923782: [PowerStateMonitor] Be conservative when determining power state
Summary

When the state defaults to AC-powered, the indexer may start some
energy consuming tasks, only to stop these later when the pending
DBus call finishes. Especially the content indexer can take a while
to stop, until the current batch is finished.

In case the DBus call fails AC-powered is assumed, to match the previous
default.

Test Plan

unplug AC power
start baloo_file
-> the content indexer no longer processes its first batch

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bruns created this revision.Jun 8 2019, 5:03 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptJun 8 2019, 5:03 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Jun 8 2019, 5:03 PM
poboiko accepted this revision.Jun 10 2019, 8:57 AM

Makes sense to me

This revision is now accepted and ready to land.Jun 10 2019, 8:57 AM
Closed by commit R293:0e6f1c923782: [PowerStateMonitor] Be conservative when determining power state (authored by bruns). · Explain WhyJun 10 2019, 10:31 AM
This revision was automatically updated to reflect the committed changes.
romangg added a subscriber: romangg.Jul 16 2019, 9:40 AM

Reverted by d6d86cb86c on master. See https://bugs.kde.org/show_bug.cgi?id=409405 for more info. Possibly a critical problem in released 5.60 @bruns.

bruns added a comment.Jul 16 2019, 10:15 AM
In D21672#496217, @romangg wrote:

Reverted by d6d86cb86c on master. See https://bugs.kde.org/show_bug.cgi?id=409405 for more info. Possibly a critical problem in released 5.60 @bruns.

Why don't you notify the relevant people first and then submit a review.

Just reverting a commit without reasoning, without review is just wrong.

broulik added a subscriber: broulik.Jul 16 2019, 10:18 AM

Why don't you notify the relevant people first

He kinda did by writing a bug report, but let's not argue but investigate and fix why this seemingly innocent patch caused the behavior described.

bruns added a comment.Jul 16 2019, 10:22 AM
In D21672#496232, @broulik wrote:

Why don't you notify the relevant people first

He kinda did by writing a bug report, but let's not argue but investigate and fix why this seemingly innocent patch caused the behavior described.

He set me as assignee today and then submitted the reversion 3 minutes later.

romangg added a comment.Jul 16 2019, 11:38 AM

The bug report was online for two weeks and it was categorized as Baloo bug. You had more than enough time to notice it.

I hoped I wouldn't need to waste any more time on this issue besides the time I already had to spent for working around this bug for two weeks, but now I had to setup baloo compilation myself, had to git bisect it and had to inform you. And now above all this unnecessary shit I have to deal with your ego being damaged when I reverted one isolated three lines patch of yours. Good news for you: the revert didn't fix the problem after reboot what I noticed now after I did one. Just when I restart baloo there is no more spam. So you can try to find the right solution now or you can continue being angry for no reason whatsoever.

Because guess what: you can revert the revert at any time or even better hand in a patch to fix the whole issue after all.

ngraham added a comment.Jul 16 2019, 5:46 PM

Let's not argue but investigate and fix why this seemingly innocent patch caused the behavior described.

Revision Contents
Changeset List

PathPackages
Msrc/file/powerstatemonitor.cpp (4 lines)

Diff 59498

View Options

src/file/powerstatemonitor.cpp

Loading...