Skip to content

Allow detecting if the disk is a SSD or HDD #28

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.

Sign up for GitHub

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

Open
h4xr wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow detecting if the disk is a SSD or HDD #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions ansible/roles/detect-ssd/tasks/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
- debug:
msg: "Checking if the disk {{ satellite_physical_disk }} is SSD"
- name: Retrieve the contents of the disk
shell:
cat "/sys/block/{{ satellite_physical_disk }}/queue/rotational
Copy link
Copy Markdown
Collaborator

@jhutar jhutar Apr 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing '"' at the end?

Copy link
Copy Markdown
Collaborator

@jhutar jhutar Apr 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given a default, this would expand into "/sys/block//dev/sda/queue/rotational" and that does not seems right

Copy link
Copy Markdown
Contributor Author

@h4xr h4xr Apr 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhutar I am a little bit confused here on which approach to take:

  • We can have a default disk set in the configuration, but that may cause mis-representation of facts if the user does not take care while running the tune utility.
  • We can have this variable set to a blank value and we can check in playbook, if the variable is blank, then don't run this detection, otherwise run this detection mechanism

register: is_ssd
- set_fact:
ssd: true
when: "{{ is_ssd.stdout | int }}" == 0
- set_fact:
ssd: false
when: "{{ is_ssd.stdout | int }}" == 1
...
3 changes: 3 additions & 0 deletions conf/sattune.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
satellite_physical_disk: /dev/sda
Copy link
Copy Markdown
Collaborator

@jhutar jhutar Apr 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we autodetect this? In lots of cases this will be on LVM and mounted from somewhere else.

Copy link
Copy Markdown
Collaborator

@jhutar jhutar Apr 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that would be probably too hard to do it right. What about simple role which would change the values + documentation for that role + having it commented out in https://github.com/redhat-performance/satellite-tune/blob/master/ansible/postgresql.yaml by default?

Copy link
Copy Markdown
Contributor Author

@h4xr h4xr Apr 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhutar I agree, this will be a much better approach, leaving the control in the hand of user, if they want to run this detection feature or not.
Taking about autodetection, though it is a bit harder approach, but we can indeed autodetect this

Copy link
Copy Markdown
Collaborator

@jhutar jhutar Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I would just call the role postgresql-on-ssd so it would be up to user to use the role or not. No auto-detection needed IMO.

...