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
Branch
scheduler
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12577
Build 12595: arc lint + arc unit
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 59409

View Options

src/file/powerstatemonitor.cpp

Loading...