New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drive power down interface #885

Closed
phillxnet opened this Issue Sep 14, 2015 · 15 comments

Comments

Projects
None yet
4 participants
@phillxnet
Member

phillxnet commented Sep 14, 2015

Currently it seems the only way to set / enable drive powerdown is via cli hdparm commands.

Now that Rockstor has hdparm by default we can add a facility to set this figure on each device.
Issues:-
Drive names change but that is dealt with at some level elsewhere I am just not sure how we translate that to the device parameter that hdparm takes, presumably via the same function that shows the drive name in Disks table.
Reasons:-
Power / noise save.
Avoid users having to enter eg /sbin/hdparm -S120 /dev/sdb above initrock -x entries in /etc/rc.local to enable 10 min power down. (a value of 239 sets a 20 mins power down)

Obvious place to add this interface might be an additional column or icon in Disks. This might then also equate to the current power state. I believe smart can relay the drives current power state so we could poll from that database but I think it better to have a dedicated entry / mechanism for power setting / state read as then lighter and can be polled without drawing in all of the smart data.

Options in this drive setting - set how long before drive enters sleep (no disk motor activity) state and an instant sleep mode option on each / all drives.

@maxhq

This comment has been minimized.

Contributor

maxhq commented Oct 22, 2015

Four thoughts about that:

1. Per-pool timeout instead of per-disk

Setting timeout for each drive might lead to confusion as the user has to check which drives belong to the same pool. Maybe it should be better set per pool (BTRFS filesystem)?
Then the corresponding drives could be fetched via btrfs fi show -d.

2. Non-rotational devices
There might be problems when spindown is configured on non-rotational devices (USD stick, SD card, SSD). I tried it on a USB stick and it became completely unaccessible (very old Linux though).

3. Don't forget, but take care with hdparm -B parameters
http://www.howtoeverything.net/linux/hardware/why-some-hard-disks-wont-spin-down-hdparm

4. Processes might spin up disks again
At least the following background processes have to be taken into consideration.
Some could be set to at least allow quiet nights, for other influences the user should at least see a hint on the future disk spindown configuration page.

  • SMART device polling - should be configured to skip disks in standby like:
    DEVICESCAN ... -n standby,48,q ...
  • Anachron - can be limited to certain hours of the day with START_HOURS_RANGE in /etc/anacrontab
  • Cron entries (/etc/cron.hourly)
  • Rockstor BTRFS snapshots (user defined)

Currently, I configure spindown times with this systemd startup script:

#!/bin/sh
# list all disks
lsblk -p -i -o KNAME,TYPE,SIZE,MODEL | while read dev type size model; do
    # only process disks (skip partitions etc.)
    test $type != "disk" && continue

    # skip non-rotational disks (indicators: rotation rate or acoustic management)
    spin_rate=`udevadm info --query=all --name=$dev | grep 'ID_ATA_ROTATION_RATE_RPM=' | cut -d= -f2`
    test -z $spin_rate && spin_rate=0
    aam=`udevadm info --query=all --name=$dev | grep 'ID_ATA_FEATURE_SET_AAM=' | cut -d= -f2`
    test "$spin_rate" == "0" -a "x$aam" != "x1" && continue

    logger -t diskspindown "Setting spindown time to 60min for $dev ($model, $size)"
    # set APM to allow spin down (ignore failures - drive might not support it)
    hdparm -B127 $dev >/dev/null 2>&1
    # set timeout to 60min
    hdparm -S242 $dev >/dev/null
done
@jakieu

This comment has been minimized.

jakieu commented Nov 3, 2015

This is something that would make me buy a 5 year subscription instantly 👍

@phillxnet

This comment has been minimized.

Member

phillxnet commented Dec 17, 2015

@maxhq Thanks for all this excellent info, I particularly like the per pool power down idea.
The drive power down issue has come up again on the forum so linking here so that it might get updated in the case of development in this area. Thanks for the script by the way, nice.

Please update the following forum thread with development on this issue: http://forum.rockstor.com/t/smart-status-and-disk-spin-down/793/7

@phillxnet

This comment has been minimized.

Member

phillxnet commented Mar 25, 2016

Taking a quick look at this again now.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

initial commit power status column rockstor#885
Currently a static placeholder. Improve comments
on columns.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Mar 26, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 2, 2016

add 'task execution time windows' reference in user info rockstor#885
Mention the (currently in development) task time window limiter
as a way to increase disk idle periods.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 2, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 4, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 4, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 4, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 4, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 4, 2016

debug logging and type use correction rockstor#885
Also updated a few older comments to help avoid further
confusion with device names.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 5, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 5, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 5, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 5, 2016

add -q to -S to suppress all but error output rockstor#885
We are simply setting the idle spin down so don't need
terminal feedback.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 5, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 5, 2016

populate get_disk_power_status rockstor#885
Improve comments on hdparm details.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 6, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 6, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 6, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 6, 2016

@phillxnet

This comment has been minimized.

Member

phillxnet commented Apr 6, 2016

Disk Power Status is on the way:
disks-power-status-column
The edit icon leads to configuring the spin down time.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 21, 2016

when APM of disk is unknown hide all APM settings rockstor#885
Also disable but don't hide the 'Enable' tickbox for these
settings to maintain affordance for these settings existence
for future visits. If we can't read them we should make no
attempt to adjust them.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 21, 2016

fix a false positive for is_rotational test rockstor#885
If we look at the AAM current value rather than just:
ID_ATA_FEATURE_SET_AAM=1
we get a more reliable indication of rotational nature.
Based on an msata device TS32GMSA370 which reports
having AAM (Automatic Acoustic Management) which was
previously assumed to be an indicator of a rotational device.
Real rotational devices appear to have a non zero:
ID_ATA_FEATURE_SET_AAM_CURRENT_VALUE via udevadm.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 21, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 21, 2016

'enable apm' check box to hide or show apm settings rockstor#885
Also add safeguard to ensure no apm setting is applied when
enable_apm is unticked. The apm 0 level is used as a flag to
our existing sub system to set this up.
@phillxnet

This comment has been minimized.

Member

phillxnet commented Apr 21, 2016

Linking to recently closed pr for collaboration purposes (closed due to deleting and recreating the github branch):
#1260
I will shortly open a fresh pr using the now re-created and rebased (on 3.8-13) branch by the same name.

@phillxnet

This comment has been minimized.

Member

phillxnet commented Apr 21, 2016

remaining issues:-

  • input validation bug caused by contention between the text box updating the slider first rather than triggering the validation routine. Will have another look shortly.
  • dealing with completely removing a drives custom config for spin-down
  • disable the associated systemd service when the config for the last drive entry is removed.
  • consider upstream changes in #1287 re disk table handlebar helpers re-factoring.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 22, 2016

add a 'remove config' option to the spin down config page rockstor#885
With this option we send a flag value of -1 (normally 1-255)
for the spindown time to indicate we want a current setting
(if any) removed rather than updated or added (if non exists).
On receiving this value we don't test the now nonsense hdparm
command, as it would make no sense, and go ahead and use the
existing mechanism to update our systemd file but this time by
removing an existing entry.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 22, 2016

Initial mechanism for disabling hdparm systemd rockstor#885
Currently buggy but commiting as save point.
Intended to disable and remove the systemd file when
no remaining ExecStart lines remain.
@phillxnet

This comment has been minimized.

Member

phillxnet commented Apr 23, 2016

Linking to disk table helper changes under issue #1275 via pr #1287 as I've added a couple of columns to the disk table in this issue.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

debug hdparm entry removal when there are none rockstor#885
Additional clauses to deal with removing non existing config
and improve file management a tad by not copying our edited
file if we are then to remove it directly afterwards. Also don't
accidentally start a service file with no entries, could happen
when creating a new but empty config via the remove function
when no config existed.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

improve wording in template rockstor-hdparm.service file rockstor#885
This is important as the file is auto edited / maintained and so
is expected to be of a certain format. The comments indicate this.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

sort out apm_value validation contention issue giving NaN rockstor#885
When we try and update the slider from a non numeric value from
our apm_value text entry the slider and text box can end up
dispalying NaN. Avoid by trapping isNaN and not passing to slider
as then our in place form validation can deal with it in a more
friendly fashion.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

remove extraneous disk table cell trailing spaces rockstor#885
Brings in line with newer format. Changing only in hdparm
and apm columns as already removed in other columns up
stream.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

prepare handlebar helper to display apm levels humanized rockstor#885
To aid with upstream changes where refactoring has split
the helper we use currently into many smaller ones like this.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

prepare conditional handlebar helpers for drive power state rockstor#885


To aid with upstream changes where refactoring has split
the helper we use currently into many smaller ones like this.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 23, 2016

@phillxnet

This comment has been minimized.

Member

phillxnet commented Apr 24, 2016

Currently doing final testing prior to submitting pull request for review.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 29, 2016

remove hard coded line count on hdparm systemd template rockstor#885
Thanks to @maxhq on GitHub for their review which lead to this
commit. Minor comment improvements also added.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 29, 2016

check for hdparm systemd template file's existence rockstor#885
As our edits rely on this template we should recognize it's
non existence and log accordingly.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Apr 29, 2016

reformat recent code to PEP8 rockstor#885
Minor surface reformatting of recently changed code.

@schakrava schakrava closed this in 211b31a May 1, 2016

@schakrava schakrava added this to the Looney Bean milestone May 1, 2016

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 2, 2016

The power down feature works great but I get the following warning from systemd when the service unit is updated:
May 2 13:08:24 Sigurd systemd[1]: Configuration file /etc/systemd/system/rockstor-hdparm.service is marked world-inaccessible. This has no effect as configuration data is accessible via APIs without restrictions. Proceeding anyway.

@phillxnet

This comment has been minimized.

Member

phillxnet commented May 2, 2016

@maxhq Thanks for the report and I'm chuffed it's working for you. We have a few similar systemd file rights warning on some other services as well. Doesn't look like it's a problem but yes this is definitely something that needs to be sorted at some point. Started happening a few releases ago and I suspect it's down to some upstream policy changes on such things.

Would you mind stating what drive models you have had success with and if they needed any non default apm settings; especially useful if the drives are a popular model. Though this discussion may well be better located on the forum in a thread of it's own, but anywhere is good really.

Thanks again for you input.

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 3, 2016

@phillxnet Sorry: bad news. Powering down only seems to work for my Seagate Barracuda ST1000DM003-1ER162 disks. I set APM to 127 (default: 128).
It does not work for Western Digital WDC WD10EARS-00Y5B1, which has no APM but an old AAM feature (but I guess these old HDDs from around 2011 are not so relevant anymore).

@phillxnet

This comment has been minimized.

Member

phillxnet commented May 3, 2016

@maxhq Well I'd rather see this as good news actually, at least it works on some drives.:) .
I have tested here on some pretty old drives (eg 80 - 360 GB examples) with some working and other not so it's a bit hit and miss. Is you experience that the setting doesn't take, ie doesn't show up in the disks table, or that it shows up but doesn't then enter standby when left idle?

There may also be some upper limits imposed by as yet undetermined factors ie a couple of
ST3000VN000-1HJ1 3TB Seagate NAS drives I have here only work on values up to 20 minutes so that another thing to try.

(Leaving a browser open on some pages of Rockstor's WebUI can inhibit spin down, this has been noticed most on the disks page so far)

Given that this issue is now closed we are likely to get more eyes on this if we continue in the forum thread that is now open to work out what might be done to improve things.
"Disk spin down does not work on some of my disks"
which in turn grew from this post: https://forum.rockstor.com/t/dev-log-for-3-8-14/1412/11?u=phillxnet
in the generic dev log thread.
There is already some diagnostic questions I've posted there that may help narrow down what's not working as well as it could.
The first thread currently addresses the first arrangement (config not showing up), as far as I'm currently aware, and I'm awaiting feedback on the referenced problem there. Please do chip in there if you think it's relevant or open another thread if that one doesn't seem to be appropriate. We can then open fresh issues here that are focused on particular findings or feature requests, ie an override to auto is_rotational but I'm not sure such things are a good idea really, hence a discussion first on the forum.

As you indicate we are not likely to safely achieve a 100% success on this but there is definitely room for improvement, especially given the length of time it's been available / in development. Also the big hw players in this field also struggle with this one. Seems many drives just don't always behave as expected.

Thanks @maxhq I think with a little more forum chat we can get more success on this one going forward and work out what needs to be issued on GitHub in manageable chunks. Please do open a forum post with specific finding on particular models as this is bound to help. I am definitely interested in refining this a little more at least, in fact that referenced thread may have already brought up an easy improvement that may be worth testing. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment