Skip to content

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

Open
h4xr wants to merge 1 commit into
redhat-performance:masterfrom
h4xr:master
Open

Allow detecting if the disk is a SSD or HDD#28
h4xr wants to merge 1 commit into
redhat-performance:masterfrom
h4xr:master

Conversation

@h4xr
Copy link
Copy Markdown
Contributor

@h4xr h4xr commented Apr 4, 2018

The PR adds the functionality for detecting if the given disk is a HDD or an SSD.
This information can be obtained from the sysfs which sets the rotational parameter to 1 for HDDs and 0 for SSDs.
The role has a backdrop of not being able to determine the correct behavior in case of RAID volumes.
Signed-off-by: Saurabh Badhwar sbadhwar@redhat.com

Signed-off-by: Saurabh Badhwar <sbadhwar@redhat.com>
Copy link
Copy Markdown
Collaborator

@jhutar jhutar left a comment

Choose a reason for hiding this comment

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

What about to ignore detection part and have simple and documented role instead?

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

Choose a reason for hiding this comment

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

Missing '"' at the end?

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

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

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

Comment thread conf/sattune.yaml
@@ -0,0 +1,3 @@
---
satellite_physical_disk: /dev/sda
Copy link
Copy Markdown
Collaborator

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

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

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

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants